From f9ed72b14b1cbcc7af5225bae09fdaa29b70e384 Mon Sep 17 00:00:00 2001 From: federico Date: Tue, 10 Mar 2026 15:11:25 +0800 Subject: [PATCH 1/2] fix: enhance ECKey and SM2 with validation and deterministic signatures --- .../tron/core/capsule/TransactionCapsule.java | 8 +- .../java/org/tron/common/crypto/ECKey.java | 153 +++++----- .../main/java/org/tron/common/crypto/Rsv.java | 5 +- .../java/org/tron/common/crypto/sm2/SM2.java | 277 +++++------------- .../org/tron/common/crypto/sm2/SM2Signer.java | 24 +- .../src/main/java/org/tron/core/Wallet.java | 10 +- .../core/config/args/WitnessInitializer.java | 9 +- .../org/tron/common/crypto/ECKeyTest.java | 8 +- .../org/tron/common/crypto/SM2KeyTest.java | 65 +++- .../config/args/WitnessInitializerTest.java | 3 - 10 files changed, 232 insertions(+), 330 deletions(-) diff --git a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java index b11c6b1e0a4..813e72be573 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java @@ -237,9 +237,9 @@ public static long checkWeight(Permission permission, List sigs, byt } HashMap addMap = new HashMap(); for (ByteString sig : sigs) { - if (sig.size() < 65) { + if (sig == null || sig.size() < 65) { throw new SignatureFormatException( - "Signature size is " + sig.size()); + "Signature is " + (sig == null ? "null" : "invalid with size " + sig.size())); } String base64 = TransactionCapsule.getBase64FromByteString(sig); byte[] address = SignUtils @@ -617,7 +617,7 @@ public void addSign(byte[] privateKey, AccountStore accountStore) .signHash(getTransactionId().getBytes()))); this.transaction = this.transaction.toBuilder().addSignature(sig).build(); } - + private static void checkPermission(int permissionId, Permission permission, Transaction.Contract contract) throws PermissionException { if (permissionId != 0) { if (permission.getType() != PermissionType.Active) { @@ -684,7 +684,7 @@ public boolean validateSignature(AccountStore accountStore, } } isVerified = true; - } + } return true; } diff --git a/crypto/src/main/java/org/tron/common/crypto/ECKey.java b/crypto/src/main/java/org/tron/common/crypto/ECKey.java index d0a6048aca1..322f6038f09 100644 --- a/crypto/src/main/java/org/tron/common/crypto/ECKey.java +++ b/crypto/src/main/java/org/tron/common/crypto/ECKey.java @@ -31,7 +31,6 @@ import java.security.interfaces.ECPublicKey; import java.security.spec.InvalidKeySpecException; import java.util.Arrays; -import java.util.Objects; import javax.annotation.Nullable; import lombok.extern.slf4j.Slf4j; import org.bouncycastle.asn1.sec.SECNamedCurves; @@ -58,6 +57,21 @@ import org.tron.common.utils.ByteArray; import org.tron.common.utils.ByteUtil; + +/** + * ECDSA signatures are mutable: for a given (R, S) pair, both (R, S) and (R, N - S mod N) are + * valid. Canonical signatures satisfy 1 <= S <= N/2, where N is the curve order (SECP256K1N). + *

+ * Reference: + * https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki#Low_S_values_in_signatures + *

+ * For the TRON network, since the transaction ID does not include the signature and can still + * guarantee the transaction uniqueness, it is not necessary to strictly enforce signature + * canonicalization. Signature verification accepts both low-S and high-S forms. + *

+ * Note: While not enforced by the protocol, using low-S signatures is recommended to prevent + * signature malleability. + */ @Slf4j(topic = "crypto") public class ECKey implements Serializable, SignInterface { @@ -67,15 +81,6 @@ public class ECKey implements Serializable, SignInterface { public static final ECDomainParameters CURVE; public static final ECParameterSpec CURVE_SPEC; - /** - * Equal to CURVE.getN().shiftRight(1), used for canonicalising the S value of a signature. ECDSA - * signatures are mutable in the sense that for a given (R, S) pair, then both (R, S) and (R, N - - * S mod N) are valid signatures. Canonical signatures are those where 1 <= S <= N/2 - * - *

See https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki - * #Low_S_values_in_signatures - */ - public static final BigInteger HALF_CURVE_ORDER; private static final BigInteger SECP256K1N = new BigInteger("fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", 16); @@ -164,10 +169,16 @@ public ECKey(SecureRandom secureRandom) { public ECKey(byte[] key, boolean isPrivateKey) { if (isPrivateKey) { + if (!isValidPrivateKey(key)) { + throw new IllegalArgumentException("Invalid private key."); + } BigInteger pk = new BigInteger(1, key); this.privKey = privateKeyFromBigInteger(pk); this.pub = CURVE.getG().multiply(pk); } else { + if (!isValidPublicKey(key)) { + throw new IllegalArgumentException("Invalid public key."); + } this.privKey = null; this.pub = CURVE.getCurve().decodePoint(key); } @@ -202,7 +213,7 @@ public ECKey(Provider provider, @Nullable PrivateKey privKey, ECPoint pub) { public ECKey(@Nullable BigInteger priv, ECPoint pub) { this( TronCastleProvider.getInstance(), - privateKeyFromBigInteger(priv), + priv == null ? null :privateKeyFromBigInteger(priv), pub ); } @@ -231,20 +242,41 @@ private static boolean isECPrivateKey(PrivateKey privKey) { /* Convert a BigInteger into a PrivateKey object */ private static PrivateKey privateKeyFromBigInteger(BigInteger priv) { - if (priv == null) { - return null; - } else { - try { - return ECKeyFactory - .getInstance(TronCastleProvider.getInstance()) - .generatePrivate(new ECPrivateKeySpec(priv, - CURVE_SPEC)); - } catch (InvalidKeySpecException ex) { - throw new AssertionError("Assumed correct key spec statically"); - } + if (!isValidPrivateKey(priv)) { + throw new IllegalArgumentException("Invalid private key."); + } + + try { + return ECKeyFactory + .getInstance(TronCastleProvider.getInstance()) + .generatePrivate(new ECPrivateKeySpec(priv, + CURVE_SPEC)); + } catch (InvalidKeySpecException ex) { + throw new AssertionError("Assumed correct key spec statically"); } } + public static boolean isValidPrivateKey(byte[] keyBytes) { + if (ByteArray.isEmpty(keyBytes)) { + return false; + } + + BigInteger key = new BigInteger(1, keyBytes); + return key.compareTo(BigInteger.ONE) >= 0 && key.compareTo(SECP256K1N) < 0; + } + + public static boolean isValidPrivateKey(BigInteger privateKey) { + if (privateKey == null) { + return false; + } + return privateKey.compareTo(BigInteger.ONE) >= 0 && privateKey.compareTo(SECP256K1N) < 0; + } + + public static boolean isValidPublicKey(byte[] keyBytes) { + return !ByteArray.isEmpty(keyBytes); + } + + /** * Utility for compressing an elliptic curve point. Returns the same point if it's already * compressed. See the ECKey class docs for a discussion of point compression. @@ -276,6 +308,10 @@ public static ECPoint decompressPoint(ECPoint compressed) { * @return - */ public static ECKey fromPrivate(BigInteger privKey) { + if (!isValidPrivateKey(privKey)) { + throw new IllegalArgumentException("Invalid private key."); + } + return new ECKey(privKey, CURVE.getG().multiply(privKey)); } @@ -286,53 +322,13 @@ public static ECKey fromPrivate(BigInteger privKey) { * @return - */ public static ECKey fromPrivate(byte[] privKeyBytes) { - if (ByteArray.isEmpty(privKeyBytes)) { - return null; + if (!isValidPrivateKey(privKeyBytes)) { + throw new IllegalArgumentException("Invalid private key."); } - return fromPrivate(new BigInteger(1, privKeyBytes)); - } - /** - * Creates an ECKey that simply trusts the caller to ensure that point is really the result of - * multiplying the generator point by the private key. This is used to speed things up when you - * know you have the right values already. The compression state of pub will be preserved. - * - * @param priv - - * @param pub - - * @return - - */ - public static ECKey fromPrivateAndPrecalculatedPublic(BigInteger priv, - ECPoint pub) { - return new ECKey(priv, pub); - } - - /** - * Creates an ECKey that simply trusts the caller to ensure that point is really the result of - * multiplying the generator point by the private key. This is used to speed things up when you - * know you have the right values already. The compression state of the point will be preserved. - * - * @param priv - - * @param pub - - * @return - - */ - public static ECKey fromPrivateAndPrecalculatedPublic(byte[] priv, byte[] - pub) { - check(priv != null, "Private key must not be null"); - check(pub != null, "Public key must not be null"); - return new ECKey(new BigInteger(1, priv), CURVE.getCurve() - .decodePoint(pub)); + return fromPrivate(new BigInteger(1, privKeyBytes)); } - /** - * Creates an ECKey that cannot be used for signing, only verifying signatures, from the given - * point. The compression state of pub will be preserved. - * - * @param pub - - * @return - - */ - public static ECKey fromPublicOnly(ECPoint pub) { - return new ECKey(null, pub); - } /** * Creates an ECKey that cannot be used for signing, only verifying signatures, from the given @@ -517,10 +513,12 @@ public static boolean isPubKeyCanonical(byte[] pubkey) { @Nullable public static byte[] recoverPubBytesFromSignature(int recId, ECDSASignature sig, byte[] messageHash) { - check(recId >= 0, "recId must be positive"); - check(sig.r.signum() >= 0, "r must be positive"); - check(sig.s.signum() >= 0, "s must be positive"); - check(messageHash != null, "messageHash must not be null"); + check(recId >= 0 && recId <= 3, "recId must be in range [0, 3]"); + check(sig.r != null && sig.r.signum() > 0 && BIUtil.isLessThan(sig.r, SECP256K1N), + "r must be in range (0, n)"); + check(sig.s != null && sig.s.signum() > 0 && BIUtil.isLessThan(sig.s, SECP256K1N), + "s must be in range (0, n)"); + check(messageHash != null && messageHash.length == 32, "messageHash must be 32 bytes"); // 1.0 For j from 0 to h (h == recId here and the loop is outside // this function) // 1.1 Let x = r + jn @@ -744,22 +742,6 @@ public String toString() { return b.toString(); } - /** - * Produce a string rendering of the ECKey INCLUDING the private key. Unless you absolutely need - * the private key it is better for security reasons to just use toString(). - * - * @return - - */ - public String toStringWithPrivate() { - StringBuilder b = new StringBuilder(); - b.append(toString()); - if (privKey != null && privKey instanceof BCECPrivateKey) { - b.append(" priv:").append(Hex.toHexString(((BCECPrivateKey) - privKey).getD().toByteArray())); - } - return b.toString(); - } - /** * Signs the given hash and returns the R and S components as BigIntegers and putData them in * ECDSASignature @@ -1019,5 +1001,4 @@ public int hashCode() { public static class MissingPrivateKeyException extends RuntimeException { } - } diff --git a/crypto/src/main/java/org/tron/common/crypto/Rsv.java b/crypto/src/main/java/org/tron/common/crypto/Rsv.java index 15c8498e836..70e1e1c05c2 100644 --- a/crypto/src/main/java/org/tron/common/crypto/Rsv.java +++ b/crypto/src/main/java/org/tron/common/crypto/Rsv.java @@ -13,8 +13,11 @@ public class Rsv { private final byte[] s; private final byte v; - public static Rsv fromSignature(byte[] sign) { + if (sign == null || sign.length < 65) { + throw new IllegalArgumentException( + "Invalid signature length: " + (sign == null ? "null" : sign.length)); + } byte[] r = Arrays.copyOfRange(sign, 0, 32); byte[] s = Arrays.copyOfRange(sign, 32, 64); byte v = sign[64]; diff --git a/crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java b/crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java index b1d349efad3..63237db9c9e 100644 --- a/crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java +++ b/crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java @@ -4,7 +4,6 @@ import static org.tron.common.utils.BIUtil.isLessThan; import static org.tron.common.utils.ByteUtil.bigIntegerToBytes; -import java.io.IOException; import java.io.Serializable; import java.math.BigInteger; import java.nio.charset.Charset; @@ -15,12 +14,8 @@ import java.security.interfaces.ECPublicKey; import java.security.spec.InvalidKeySpecException; import java.util.Arrays; -import java.util.Objects; import javax.annotation.Nullable; import lombok.extern.slf4j.Slf4j; -import org.bouncycastle.asn1.ASN1InputStream; -import org.bouncycastle.asn1.ASN1Integer; -import org.bouncycastle.asn1.DLSequence; import org.bouncycastle.asn1.x9.X9IntegerConverter; import org.bouncycastle.crypto.AsymmetricCipherKeyPair; import org.bouncycastle.crypto.generators.ECKeyPairGenerator; @@ -41,11 +36,12 @@ import org.tron.common.crypto.SignatureInterface; import org.tron.common.crypto.jce.ECKeyFactory; import org.tron.common.crypto.jce.TronCastleProvider; +import org.tron.common.utils.BIUtil; import org.tron.common.utils.ByteArray; import org.tron.common.utils.ByteUtil; /** - * Implement Chinese Commercial Cryptographic Standard of SM2 + * This is a non-standard SM2 signature implementation adapted for blockchain scenario. */ @Slf4j(topic = "crypto") public class SM2 implements Serializable, SignInterface { @@ -63,20 +59,19 @@ public class SM2 implements Serializable, SignInterface { private static BigInteger SM2_GY = new BigInteger( "BC3736A2F4F6779C59BDCEE36B692153D0A9877CC62A474002DF32E52139F0A0", 16); - private static ECDomainParameters ecc_param; - private static ECParameterSpec ecc_spec; + private static ECDomainParameters eccParam; + private static ECParameterSpec eccSpec; private static ECCurve.Fp curve; - private static ECPoint ecc_point_g; + private static ECPoint eccPointG; private static final SecureRandom secureRandom; - static { secureRandom = new SecureRandom(); curve = new ECCurve.Fp(SM2_P, SM2_A, SM2_B, null, null); - ecc_point_g = curve.createPoint(SM2_GX, SM2_GY); - ecc_param = new ECDomainParameters(curve, ecc_point_g, SM2_N); - ecc_spec = new ECParameterSpec(curve, ecc_point_g, SM2_N); + eccPointG = curve.createPoint(SM2_GX, SM2_GY); + eccParam = new ECDomainParameters(curve, eccPointG, SM2_N); + eccSpec = new ECParameterSpec(curve, eccPointG, SM2_N); } protected final ECPoint pub; @@ -106,7 +101,7 @@ public SM2() { */ public SM2(SecureRandom secureRandom) { - ECKeyGenerationParameters ecKeyGenerationParameters = new ECKeyGenerationParameters(ecc_param, + ECKeyGenerationParameters ecKeyGenerationParameters = new ECKeyGenerationParameters(eccParam, secureRandom); ECKeyPairGenerator keyPairGenerator = new ECKeyPairGenerator(); keyPairGenerator.init(ecKeyGenerationParameters); @@ -121,16 +116,23 @@ public SM2(SecureRandom secureRandom) { public SM2(byte[] key, boolean isPrivateKey) { if (isPrivateKey) { + if (!isValidPrivateKey(key)) { + throw new IllegalArgumentException("Invalid private key in SM2."); + } + BigInteger pk = new BigInteger(1, key); this.privKey = privateKeyFromBigInteger(pk); - this.pub = ecc_param.getG().multiply(pk); + this.pub = eccParam.getG().multiply(pk); } else { + if (ByteArray.isEmpty(key)) { + throw new IllegalArgumentException("Empty public key in SM2."); + } + this.privKey = null; - this.pub = ecc_param.getCurve().decodePoint(key); + this.pub = eccParam.getCurve().decodePoint(key); } } - /** * Pair a private key with a public EC point. * @@ -171,17 +173,17 @@ public SM2(@Nullable BigInteger priv, ECPoint pub) { * Convert a BigInteger into a PrivateKey object */ private static PrivateKey privateKeyFromBigInteger(BigInteger priv) { - if (priv == null) { - return null; - } else { - try { - return ECKeyFactory - .getInstance(TronCastleProvider.getInstance()) - .generatePrivate(new ECPrivateKeySpec(priv, - ecc_spec)); - } catch (InvalidKeySpecException ex) { - throw new AssertionError("Assumed correct key spec statically"); - } + if (!isValidPrivateKey(priv)) { + throw new IllegalArgumentException("Invalid private key in SM2."); + } + + try { + return ECKeyFactory + .getInstance(TronCastleProvider.getInstance()) + .generatePrivate(new ECPrivateKeySpec(priv, + eccSpec)); + } catch (InvalidKeySpecException ex) { + throw new AssertionError("Assumed correct key spec statically"); } } @@ -203,7 +205,7 @@ private static ECPoint extractPublicKey(final ECPublicKey ecPublicKey) { final BigInteger xCoord = publicPointW.getAffineX(); final BigInteger yCoord = publicPointW.getAffineY(); - return ecc_param.getCurve().createPoint(xCoord, yCoord); + return eccParam.getCurve().createPoint(xCoord, yCoord); } @@ -216,7 +218,7 @@ private static ECPoint extractPublicKey(final ECPublicKey ecPublicKey) { * @deprecated per-point compression property will be removed in Bouncy Castle */ public static ECPoint compressPoint(ECPoint uncompressed) { - return ecc_param.getCurve().decodePoint(uncompressed.getEncoded(true)); + return eccParam.getCurve().decodePoint(uncompressed.getEncoded(true)); } /** @@ -228,7 +230,7 @@ public static ECPoint compressPoint(ECPoint uncompressed) { * @deprecated per-point compression property will be removed in Bouncy Castle */ public static ECPoint decompressPoint(ECPoint compressed) { - return ecc_param.getCurve().decodePoint(compressed.getEncoded(false)); + return eccParam.getCurve().decodePoint(compressed.getEncoded(false)); } /** @@ -238,7 +240,11 @@ public static ECPoint decompressPoint(ECPoint compressed) { * @return - */ public static SM2 fromPrivate(BigInteger privKey) { - return new SM2(privKey, ecc_param.getG().multiply(privKey)); + if (!isValidPrivateKey(privKey)) { + throw new IllegalArgumentException("Invalid private key in SM2."); + } + + return new SM2(privKey, eccParam.getG().multiply(privKey)); } /** @@ -248,52 +254,27 @@ public static SM2 fromPrivate(BigInteger privKey) { * @return - */ public static SM2 fromPrivate(byte[] privKeyBytes) { - if (ByteArray.isEmpty(privKeyBytes)) { - return null; + if (!isValidPrivateKey(privKeyBytes)) { + throw new IllegalArgumentException("Invalid private key in SM2."); } + return fromPrivate(new BigInteger(1, privKeyBytes)); } - /** - * Creates an SM2 that simply trusts the caller to ensure that point is really the result of - * multiplying the generator point by the private key. This is used to speed things up when you - * know you have the right values already. The compression state of pub will be preserved. - * - * @param priv - - * @param pub - - * @return - - */ - public static SM2 fromPrivateAndPrecalculatedPublic(BigInteger priv, - ECPoint pub) { - return new SM2(priv, pub); - } + public static boolean isValidPrivateKey(byte[] keyBytes) { + if (ByteArray.isEmpty(keyBytes)) { + return false; + } - /** - * Creates an SM2 that simply trusts the caller to ensure that point is really the result of - * multiplying the generator point by the private key. This is used to speed things up when you - * know you have the right values already. The compression state of the point will be preserved. - * - * @param priv - - * @param pub - - * @return - - */ - public static SM2 fromPrivateAndPrecalculatedPublic(byte[] priv, byte[] - pub) { - check(priv != null, "Private key must not be null"); - check(pub != null, "Public key must not be null"); - return new SM2(new BigInteger(1, priv), ecc_param.getCurve() - .decodePoint(pub)); + BigInteger key = new BigInteger(1, keyBytes); + return key.compareTo(BigInteger.ONE) >= 0 && key.compareTo(SM2_N) < 0; } - /** - * Creates an SM2 that cannot be used for signing, only verifying signatures, from the given - * point. The compression state of pub will be preserved. - * - * @param pub - - * @return - - */ - public static SM2 fromPublicOnly(ECPoint pub) { - return new SM2((PrivateKey) null, pub); + public static boolean isValidPrivateKey(BigInteger privateKey) { + if (privateKey == null) { + return false; + } + return privateKey.compareTo(BigInteger.ONE) >= 0 && privateKey.compareTo(SM2_N) < 0; } /** @@ -304,7 +285,7 @@ public static SM2 fromPublicOnly(ECPoint pub) { * @return - */ public static SM2 fromPublicOnly(byte[] pub) { - return new SM2((PrivateKey) null, ecc_param.getCurve().decodePoint(pub)); + return new SM2((PrivateKey) null, eccParam.getCurve().decodePoint(pub)); } /** @@ -317,7 +298,7 @@ public static SM2 fromPublicOnly(byte[] pub) { */ public static byte[] publicKeyFromPrivate(BigInteger privKey, boolean compressed) { - ECPoint point = ecc_param.getG().multiply(privKey); + ECPoint point = eccParam.getG().multiply(privKey); return point.getEncoded(compressed); } @@ -590,7 +571,7 @@ public SM2Signature signMsg(byte[] msg, @Nullable String userID) { private SM2Signer getSigner() { SM2Signer signer = new SM2Signer(); BigInteger d = getPrivKey(); - ECPrivateKeyParameters privateKeyParameters = new ECPrivateKeyParameters(d, ecc_param); + ECPrivateKeyParameters privateKeyParameters = new ECPrivateKeyParameters(d, eccParam); signer.init(true, privateKeyParameters); return signer; } @@ -600,7 +581,7 @@ private SM2Signer getSigner() { */ public SM2Signer getSM2SignerForHash() { SM2Signer signer = new SM2Signer(); - ECPublicKeyParameters publicKeyParameters = new ECPublicKeyParameters(pub, ecc_param); + ECPublicKeyParameters publicKeyParameters = new ECPublicKeyParameters(pub, eccParam); signer.init(false, publicKeyParameters); return signer; } @@ -614,14 +595,16 @@ public SM2Signer getSM2SignerForHash() { public static byte[] recoverPubBytesFromSignature(int recId, SM2Signature sig, byte[] messageHash) { - check(recId >= 0, "recId must be positive"); - check(sig.r.signum() >= 0, "r must be positive"); - check(sig.s.signum() >= 0, "s must be positive"); - check(messageHash != null, "messageHash must not be null"); + check(recId >= 0 && recId <= 3, "recId must be in range [0, 3]"); + check(sig.r != null && sig.r.signum() > 0 && BIUtil.isLessThan(sig.r, SM2_N), + "r must be in range (0, n)"); + check(sig.s != null && sig.s.signum() > 0 && BIUtil.isLessThan(sig.s, SM2_N), + "s must be in range (0, n)"); + check(messageHash != null && messageHash.length == 32, "messageHash must be 32 bytes"); // 1.0 For j from 0 to h (h == recId here and the loop is outside // this function) // 1.1 Let x = r + jn - BigInteger n = ecc_param.getN(); // Curve order. + BigInteger n = eccParam.getN(); // Curve order. BigInteger prime = curve.getQ(); BigInteger i = BigInteger.valueOf((long) recId / 2); @@ -640,7 +623,7 @@ public static byte[] recoverPubBytesFromSignature(int recId, // // More concisely, what these points mean is to use X as a compressed // public key. - ECCurve.Fp curve = (ECCurve.Fp) ecc_param.getCurve(); + ECCurve.Fp curve = (ECCurve.Fp) eccParam.getCurve(); // Bouncy Castle is not consistent // about the letter it uses for the prime. if (x.compareTo(prime) >= 0) { @@ -659,11 +642,15 @@ public static byte[] recoverPubBytesFromSignature(int recId, } // recover Q from the formula: s*G + (s+r)*Q = R => Q = (s+r)^(-1) (R-s*G) - BigInteger srInv = sig.s.add(sig.r).modInverse(n); + BigInteger sAddR = sig.s.add(sig.r).mod(n); + if (sAddR.equals(BigInteger.ZERO)) { + return null; + } + BigInteger srInv = sAddR.modInverse(n); BigInteger sNeg = BigInteger.ZERO.subtract(sig.s).mod(n); BigInteger coeff = srInv.multiply(sNeg).mod(n); - ECPoint.Fp q = (ECPoint.Fp) ECAlgorithms.sumOfTwoMultiplies(ecc_param + ECPoint.Fp q = (ECPoint.Fp) ECAlgorithms.sumOfTwoMultiplies(eccParam .getG(), coeff, R, srInv); return q.getEncoded(/* compressed */ false); } @@ -678,10 +665,10 @@ public static byte[] recoverPubBytesFromSignature(int recId, private static ECPoint decompressKey(BigInteger xBN, boolean yBit) { X9IntegerConverter x9 = new X9IntegerConverter(); - byte[] compEnc = x9.integerToBytes(xBN, 1 + x9.getByteLength(ecc_param + byte[] compEnc = x9.integerToBytes(xBN, 1 + x9.getByteLength(eccParam .getCurve())); compEnc[0] = (byte) (yBit ? 0x03 : 0x02); - return ecc_param.getCurve().decodePoint(compEnc); + return eccParam.getCurve().decodePoint(compEnc); } private static void check(boolean test, String message) { @@ -703,8 +690,8 @@ private static void check(boolean test, String message) { public static boolean verify(byte[] data, SM2Signature signature, byte[] pub) { SM2Signer signer = new SM2Signer(); - ECPublicKeyParameters params = new ECPublicKeyParameters(ecc_param - .getCurve().decodePoint(pub), ecc_param); + ECPublicKeyParameters params = new ECPublicKeyParameters(eccParam + .getCurve().decodePoint(pub), eccParam); signer.init(false, params); try { return signer.verifyHashSignature(data, signature.r, signature.s); @@ -718,58 +705,6 @@ public static boolean verify(byte[] data, SM2Signature signature, } } - /** - * Verifies the given ASN.1 encoded SM2 signature against a hash using the public key. - * - * @param data Hash of the data to verify. - * @param signature signature. - * @param pub The public key bytes to use. - * @return - - */ - public static boolean verify(byte[] data, byte[] signature, byte[] pub) { - return verify(data, SM2Signature.decodeFromDER(signature), pub); - } - - /** - *

Verifies the given SM2 signature against the message bytes using the public key bytes. - * - * @param msg the message data to verify. - * @param signature signature. - * @param pub The public key bytes to use. - * @return - - */ - public static boolean verifyMessage(byte[] msg, SM2Signature signature, - byte[] pub, @Nullable String userID) { - SM2Signer signer = new SM2Signer(); - ECPublicKeyParameters params = new ECPublicKeyParameters(ecc_param - .getCurve().decodePoint(pub), ecc_param); - signer.init(false, params); - try { - return signer.verifySignature(msg, signature.r, signature.s, userID); - } catch (NullPointerException npe) { - // Bouncy Castle contains a bug that can cause NPEs given - // specially crafted signatures. - // Those signatures are inherently invalid/attack sigs so we just - // fail them here rather than crash the thread. - logger.error("Caught NPE inside bouncy castle", npe); - return false; - } - } - - /** - * Verifies the given ASN.1 encoded SM2 signature against a hash using the public key. - * - * @param msg the message data to verify. - * @param signature signature. - * @param pub The public key bytes to use. - * @return - - */ - public static boolean verifyMessage(byte[] msg, byte[] signature, byte[] pub, - @Nullable String userID) { - return verifyMessage(msg, SM2Signature.decodeFromDER(signature), pub, userID); - } - - /** * Returns true if the given pubkey is canonical, i.e. the correct length taking into account * compression. @@ -890,33 +825,6 @@ public String toString() { return b.toString(); } - /** - * Produce a string rendering of the ECKey INCLUDING the private key. Unless you absolutely need - * the private key it is better for security reasons to just use toString(). - * - * @return - - */ - public String toStringWithPrivate() { - StringBuilder b = new StringBuilder(); - b.append(toString()); - if (privKey != null && privKey instanceof BCECPrivateKey) { - b.append(" priv:").append(Hex.toHexString(((BCECPrivateKey) - privKey).getD().toByteArray())); - } - return b.toString(); - } - - /** - * Verifies the given ASN.1 encoded SM2 signature against a hash using the public key. - * - * @param data Hash of the data to verify. - * @param signature signature. - * @return - - */ - public boolean verify(byte[] data, byte[] signature) { - return SM2.verify(data, signature, getPubKey()); - } - /** * Verifies the given R/S pair (signature) against a hash using the public key. * @@ -1048,46 +956,10 @@ public static boolean validateComponents(BigInteger r, BigInteger s, return isLessThan(s, SM2.SM2_N); } - public static SM2Signature decodeFromDER(byte[] bytes) { - ASN1InputStream decoder = null; - try { - decoder = new ASN1InputStream(bytes); - DLSequence seq = (DLSequence) decoder.readObject(); - if (seq == null) { - throw new RuntimeException("Reached past end of ASN.1 " - + "stream."); - } - ASN1Integer r, s; - try { - r = (ASN1Integer) seq.getObjectAt(0); - s = (ASN1Integer) seq.getObjectAt(1); - } catch (ClassCastException e) { - throw new IllegalArgumentException(e); - } - // OpenSSL deviates from the DER spec by interpreting these - // values as unsigned, though they should not be - // Thus, we always use the positive versions. See: - // http://r6.ca/blog/20111119T211504Z.html - return new SM2Signature(r.getPositiveValue(), s - .getPositiveValue()); - } catch (IOException e) { - throw new RuntimeException(e); - } finally { - if (decoder != null) { - try { - decoder.close(); - } catch (IOException x) { - - } - } - } - } - public boolean validateComponents() { return validateComponents(r, s, v); } - /** * @return - */ @@ -1140,5 +1012,4 @@ public int hashCode() { return result; } } - } diff --git a/crypto/src/main/java/org/tron/common/crypto/sm2/SM2Signer.java b/crypto/src/main/java/org/tron/common/crypto/sm2/SM2Signer.java index 817b909de58..35a0d72471d 100644 --- a/crypto/src/main/java/org/tron/common/crypto/sm2/SM2Signer.java +++ b/crypto/src/main/java/org/tron/common/crypto/sm2/SM2Signer.java @@ -11,9 +11,8 @@ import org.bouncycastle.crypto.params.ECPrivateKeyParameters; import org.bouncycastle.crypto.params.ECPublicKeyParameters; import org.bouncycastle.crypto.params.ParametersWithID; -import org.bouncycastle.crypto.params.ParametersWithRandom; import org.bouncycastle.crypto.signers.DSAKCalculator; -import org.bouncycastle.crypto.signers.RandomDSAKCalculator; +import org.bouncycastle.crypto.signers.HMacDSAKCalculator; import org.bouncycastle.math.ec.ECConstants; import org.bouncycastle.math.ec.ECFieldElement; import org.bouncycastle.math.ec.ECMultiplier; @@ -24,7 +23,7 @@ public class SM2Signer implements ECConstants { - private final DSAKCalculator kCalculator = new RandomDSAKCalculator(); + private final DSAKCalculator kCalculator = new HMacDSAKCalculator(new SM3Digest()); private byte[] userID; @@ -46,22 +45,12 @@ public void init(boolean forSigning, CipherParameters param) { userID = new byte[0]; } + ecKey = (ECKeyParameters) baseParam; + ecParams = ecKey.getParameters(); + if (forSigning) { - if (baseParam instanceof ParametersWithRandom) { - ParametersWithRandom rParam = (ParametersWithRandom) baseParam; - - ecKey = (ECKeyParameters) rParam.getParameters(); - ecParams = ecKey.getParameters(); - kCalculator.init(ecParams.getN(), rParam.getRandom()); - } else { - ecKey = (ECKeyParameters) baseParam; - ecParams = ecKey.getParameters(); - kCalculator.init(ecParams.getN(), new SecureRandom()); - } pubPoint = ecParams.getG().multiply(((ECPrivateKeyParameters) ecKey).getD()).normalize(); } else { - ecKey = (ECKeyParameters) baseParam; - ecParams = ecKey.getParameters(); pubPoint = ((ECPublicKeyParameters) ecKey).getQ(); } @@ -114,6 +103,9 @@ public BigInteger[] generateHashSignature(byte[] hash) { ECMultiplier basePointMultiplier = createBasePointMultiplier(); + // Initialize the deterministic K calculator with the private key and message hash + kCalculator.init(n, d, hash); + // 5.2.1 Draft RFC: SM2 Public Key Algorithms do // generate s { diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index 8c86f2f66ac..37caff87788 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -651,9 +651,9 @@ public TransactionApprovedList getTransactionApprovedList(Transaction trx) { byte[] hash = Sha256Hash.hash(CommonParameter .getInstance().isECKeyCryptoEngine(), trx.getRawData().toByteArray()); for (ByteString sig : trx.getSignatureList()) { - if (sig.size() < 65) { + if (sig == null || sig.size() < 65) { throw new SignatureFormatException( - "Signature size is " + sig.size()); + "Signature is " + (sig == null ? "null" : "invalid with size " + sig.size())); } String base64 = TransactionCapsule.getBase64FromByteString(sig); byte[] address = SignUtils.signatureToAddress(hash, base64, Args.getInstance() @@ -780,7 +780,7 @@ public WitnessList getPaginatedNowWitnessList(long offset, long limit) throws if (limit > WITNESS_COUNT_LIMIT_MAX) { limit = WITNESS_COUNT_LIMIT_MAX; } - + /* In the maintenance period, the VoteStores will be cleared. To avoid the race condition of VoteStores deleted but Witness vote counts not updated, @@ -1502,8 +1502,8 @@ public Protocol.ChainParameters getChainParameters() { builder.addChainParameter(Protocol.ChainParameters.ChainParameter.newBuilder() .setKey("getAllowTvmSelfdestructRestriction") .setValue(dbManager.getDynamicPropertiesStore().getAllowTvmSelfdestructRestriction()) - .build()); - + .build()); + builder.addChainParameter(Protocol.ChainParameters.ChainParameter.newBuilder() .setKey("getProposalExpireTime") .setValue(dbManager.getDynamicPropertiesStore().getProposalExpireTime()) diff --git a/framework/src/main/java/org/tron/core/config/args/WitnessInitializer.java b/framework/src/main/java/org/tron/core/config/args/WitnessInitializer.java index 2a97b6ab631..f51a1f6d982 100644 --- a/framework/src/main/java/org/tron/core/config/args/WitnessInitializer.java +++ b/framework/src/main/java/org/tron/core/config/args/WitnessInitializer.java @@ -7,6 +7,7 @@ import java.util.List; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; +import org.tron.common.crypto.ECKey; import org.tron.common.crypto.SignInterface; import org.tron.common.utils.ByteArray; import org.tron.common.utils.Commons; @@ -111,8 +112,12 @@ private void tryInitFromKeystore() { Credentials credentials = WalletUtils .loadCredentials(password, new File(fileName)); SignInterface sign = credentials.getSignInterface(); - String prikey = ByteArray.toHexString(sign.getPrivateKey()); - privateKeys.add(prikey); + byte[] privateKeyBytes = sign.getPrivateKey(); + if (!ECKey.isValidPrivateKey(privateKeyBytes)) { + throw new TronError("Private key from keystore is invalid", + TronError.ErrCode.WITNESS_KEYSTORE_LOAD); + } + privateKeys.add(ByteArray.toHexString(privateKeyBytes)); } catch (IOException | CipherException e) { logger.error("Witness node start failed!"); throw new TronError(e, TronError.ErrCode.WITNESS_KEYSTORE_LOAD); diff --git a/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java b/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java index 273672e8342..e9cf3418079 100644 --- a/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java +++ b/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java @@ -5,7 +5,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.tron.common.utils.client.utils.AbiUtil.generateOccupationConstantPrivateKey; @@ -69,10 +69,8 @@ public void testFromPrivateKey() { assertTrue(key.hasPrivKey()); assertArrayEquals(pubKey, key.getPubKey()); - key = ECKey.fromPrivate((byte[]) null); - assertNull(key); - key = ECKey.fromPrivate(new byte[0]); - assertNull(key); + assertThrows(IllegalArgumentException.class, () -> ECKey.fromPrivate((byte[]) null)); + assertThrows(IllegalArgumentException.class, () -> ECKey.fromPrivate(new byte[0])); } @Test(expected = IllegalArgumentException.class) diff --git a/framework/src/test/java/org/tron/common/crypto/SM2KeyTest.java b/framework/src/test/java/org/tron/common/crypto/SM2KeyTest.java index 87e4e14698c..11fe92bb62b 100644 --- a/framework/src/test/java/org/tron/common/crypto/SM2KeyTest.java +++ b/framework/src/test/java/org/tron/common/crypto/SM2KeyTest.java @@ -4,7 +4,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.tron.common.utils.client.utils.AbiUtil.generateOccupationConstantPrivateKey; @@ -66,10 +66,8 @@ public void testFromPrivateKey() { assertTrue(key.hasPrivKey()); assertArrayEquals(pubKey, key.getPubKey()); - key = SM2.fromPrivate((byte[]) null); - assertNull(key); - key = SM2.fromPrivate(new byte[0]); - assertNull(key); + assertThrows(IllegalArgumentException.class, () -> SM2.fromPrivate((byte[]) null)); + assertThrows(IllegalArgumentException.class, () -> SM2.fromPrivate(new byte[0])); } @Test(expected = IllegalArgumentException.class) @@ -281,4 +279,61 @@ public void testSM3() { assertEquals("b524f552cd82b8b028476e005c377fb19a87e6fc682d48bb5d42e3d9b9effe76", Hex.toHexString(eHash)); } + + /** + * Verifies SM2 signatures are deterministic with HMacDSAKCalculator (RFC 6979). + */ + @Test + public void testDeterministicSignature() { + SM2 key = SM2.fromPrivate(privateKey); + byte[] hash = Hex.decode("B524F552CD82B8B028476E005C377FB" + + "19A87E6FC682D48BB5D42E3D9B9EFFE76"); + + SM2.SM2Signature signature1 = key.sign(hash); + SM2.SM2Signature signature2 = key.sign(hash); + SM2.SM2Signature signature3 = key.sign(hash); + + assertEquals(signature1.r, signature2.r); + assertEquals(signature1.s, signature2.s); + assertEquals(signature2.r, signature3.r); + assertEquals(signature2.s, signature3.s); + } + + /** + * Test that different messages produce different signatures. + */ + @Test + public void testDifferentMessagesSignatures() { + SM2 key = SM2.fromPrivate(privateKey); + byte[] hash1 = Hex.decode("B524F552CD82B8B028476E005C377FB" + + "19A87E6FC682D48BB5D42E3D9B9EFFE76"); + byte[] hash2 = Hex.decode("C524F552CD82B8B028476E005C377FB" + + "19A87E6FC682D48BB5D42E3D9B9EFFE77"); + + SM2.SM2Signature signature1 = key.sign(hash1); + SM2.SM2Signature signature2 = key.sign(hash2); + + // Different messages should produce different signatures + // (with overwhelming probability) + assertFalse("Different messages should produce different signatures", + signature1.r.equals(signature2.r) && signature1.s.equals(signature2.s)); + } + + /** + * Test that signature verification still works correctly with deterministic signatures. + */ + @Test + public void testSignatureVerification() { + SM2 key = SM2.fromPrivate(privateKey); + String message = "Hello, SM2 deterministic signature test!"; + byte[] hash = key.getSM2SignerForHash().generateSM3Hash(message.getBytes()); + + SM2.SM2Signature signature = key.sign(hash); + + // Verify the signature + SM2Signer verifier = key.getSM2SignerForHash(); + boolean isValid = verifier.verifyHashSignature(hash, signature.r, signature.s); + + assertTrue("Signature should be valid", isValid); + } } diff --git a/framework/src/test/java/org/tron/core/config/args/WitnessInitializerTest.java b/framework/src/test/java/org/tron/core/config/args/WitnessInitializerTest.java index b38286ca84d..6e98d0b9a5a 100644 --- a/framework/src/test/java/org/tron/core/config/args/WitnessInitializerTest.java +++ b/framework/src/test/java/org/tron/core/config/args/WitnessInitializerTest.java @@ -228,9 +228,6 @@ public void testTryInitFromKeyStore2() mockedByteArray.when(() -> ByteArray.fromHexString(anyString())).thenReturn(keyBytes); witnessInitializer = new WitnessInitializer(mockConfig); - Field localWitnessField = WitnessInitializer.class.getDeclaredField("localWitnesses"); - localWitnessField.setAccessible(true); - localWitnessField.set(witnessInitializer, new LocalWitnesses(privateKey)); LocalWitnesses localWitnesses = witnessInitializer.initLocalWitnesses(); assertFalse(localWitnesses.getPrivateKeys().isEmpty()); } From dba252778b3ce2d416d7013a91d0692cd0c7bbde Mon Sep 17 00:00:00 2001 From: federico Date: Tue, 10 Mar 2026 20:56:28 +0800 Subject: [PATCH 2/2] fix(crypto): improve ECKey and SM2 robustness and code quality --- .../java/org/tron/common/crypto/ECKey.java | 81 ++++++------ .../java/org/tron/common/crypto/sm2/SM2.java | 118 ++++++------------ 2 files changed, 82 insertions(+), 117 deletions(-) diff --git a/crypto/src/main/java/org/tron/common/crypto/ECKey.java b/crypto/src/main/java/org/tron/common/crypto/ECKey.java index 322f6038f09..463eca7d393 100644 --- a/crypto/src/main/java/org/tron/common/crypto/ECKey.java +++ b/crypto/src/main/java/org/tron/common/crypto/ECKey.java @@ -19,7 +19,7 @@ import java.io.Serializable; import java.math.BigInteger; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.PrivateKey; @@ -31,6 +31,7 @@ import java.security.interfaces.ECPublicKey; import java.security.spec.InvalidKeySpecException; import java.util.Arrays; +import java.util.Objects; import javax.annotation.Nullable; import lombok.extern.slf4j.Slf4j; import org.bouncycastle.asn1.sec.SECNamedCurves; @@ -82,8 +83,7 @@ public class ECKey implements Serializable, SignInterface { public static final ECParameterSpec CURVE_SPEC; public static final BigInteger HALF_CURVE_ORDER; - private static final BigInteger SECP256K1N = - new BigInteger("fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", 16); + private static final BigInteger SECP256K1N; private static final SecureRandom secureRandom; private static final long serialVersionUID = -728224901792295832L; @@ -94,6 +94,7 @@ public class ECKey implements Serializable, SignInterface { params.getN(), params.getH()); CURVE_SPEC = new ECParameterSpec(params.getCurve(), params.getG(), params.getN(), params.getH()); + SECP256K1N = params.getN(); HALF_CURVE_ORDER = params.getN().shiftRight(1); secureRandom = new SecureRandom(); } @@ -113,8 +114,8 @@ public class ECKey implements Serializable, SignInterface { private final Provider provider; // Transient because it's calculated on demand. - private transient byte[] pubKeyHash; - private transient byte[] nodeId; + private transient volatile byte[] pubKeyHash; + private transient volatile byte[] nodeId; /** * Generates an entirely new keypair. @@ -194,13 +195,13 @@ public ECKey(Provider provider, @Nullable PrivateKey privKey, ECPoint pub) { throw new IllegalArgumentException( "Expected EC private key, given a private key object with" + " class " - + privKey.getClass().toString() + + + privKey.getClass() + " and algorithm " + privKey.getAlgorithm()); } if (pub == null) { - throw new IllegalArgumentException("Public key may not be null"); + throw new IllegalArgumentException("Public key should not be null"); } else { this.pub = pub; } @@ -273,7 +274,15 @@ public static boolean isValidPrivateKey(BigInteger privateKey) { } public static boolean isValidPublicKey(byte[] keyBytes) { - return !ByteArray.isEmpty(keyBytes); + if (ByteArray.isEmpty(keyBytes)) { + return false; + } + try { + ECPoint point = CURVE.getCurve().decodePoint(keyBytes); + return !point.isInfinity() && point.isValid(); + } catch (Exception e) { + return false; + } } @@ -477,6 +486,9 @@ public static ECKey signatureToKey(byte[] messageHash, String * @return - */ public static boolean isPubKeyCanonical(byte[] pubkey) { + if (pubkey == null || pubkey.length == 0) { + return false; + } if (pubkey[0] == 0x04) { // Uncompressed pubkey return pubkey.length == 65; @@ -522,9 +534,8 @@ public static byte[] recoverPubBytesFromSignature(int recId, // 1.0 For j from 0 to h (h == recId here and the loop is outside // this function) // 1.1 Let x = r + jn - BigInteger n = CURVE.getN(); // Curve order. BigInteger i = BigInteger.valueOf((long) recId / 2); - BigInteger x = sig.r.add(i.multiply(n)); + BigInteger x = sig.r.add(i.multiply(SECP256K1N)); // 1.2. Convert the integer x to an octet string X of length mlen // using the conversion routine // specified in Section 2.3.7, where mlen = ⌈(log2 p)/8⌉ or @@ -551,7 +562,7 @@ public static byte[] recoverPubBytesFromSignature(int recId, ECPoint R = decompressKey(x, (recId & 1) == 1); // 1.4. If nR != point at infinity, then do another iteration of // Step 1 (callers responsibility). - if (!R.multiply(n).isInfinity()) { + if (!R.multiply(SECP256K1N).isInfinity()) { return null; } // 1.5. Compute e from M using Steps 2 and 3 of ECDSA signature @@ -574,10 +585,10 @@ public static byte[] recoverPubBytesFromSignature(int recId, // taking the mod. For example the additive // inverse of 3 modulo 11 is 8 because 3 + 8 mod 11 = 0, and -3 mod // 11 = 8. - BigInteger eInv = BigInteger.ZERO.subtract(e).mod(n); - BigInteger rInv = sig.r.modInverse(n); - BigInteger srInv = rInv.multiply(sig.s).mod(n); - BigInteger eInvrInv = rInv.multiply(eInv).mod(n); + BigInteger eInv = BigInteger.ZERO.subtract(e).mod(SECP256K1N); + BigInteger rInv = sig.r.modInverse(SECP256K1N); + BigInteger srInv = rInv.multiply(sig.s).mod(SECP256K1N); + BigInteger eInvrInv = rInv.multiply(eInv).mod(SECP256K1N); ECPoint.Fp q = (ECPoint.Fp) ECAlgorithms.sumOfTwoMultiplies(CURVE .getG(), eInvrInv, R, srInv); return q.getEncoded(/* compressed */ false); @@ -670,7 +681,7 @@ public byte[] getAddress() { if (pubKeyHash == null) { pubKeyHash = Hash.computeAddress(this.pub); } - return pubKeyHash; + return Arrays.copyOf(pubKeyHash, pubKeyHash.length); } @Override @@ -692,7 +703,7 @@ public byte[] getNodeId() { if (nodeId == null) { nodeId = pubBytesWithoutFormat(this.pub); } - return nodeId; + return Arrays.copyOf(nodeId, nodeId.length); } @@ -737,9 +748,7 @@ public BigInteger getPrivKey() { } public String toString() { - StringBuilder b = new StringBuilder(); - b.append("pub:").append(Hex.toHexString(pub.getEncoded(false))); - return b.toString(); + return "pub:" + Hex.toHexString(pub.getEncoded(false)); } /** @@ -781,23 +790,20 @@ public ECDSASignature doSign(byte[] input) { */ public ECDSASignature sign(byte[] messageHash) { ECDSASignature sig = doSign(messageHash); - // Now we have to work backwards to figure out the recId needed to - // recover the signature. - int recId = -1; + sig.v = (byte) (findRecId(sig, messageHash) + 27); + return sig; + } + + private int findRecId(ECDSASignature sig, byte[] messageHash) { byte[] thisKey = this.pub.getEncoded(/* compressed */ false); for (int i = 0; i < 4; i++) { byte[] k = ECKey.recoverPubBytesFromSignature(i, sig, messageHash); if (k != null && Arrays.equals(k, thisKey)) { - recId = i; - break; + return i; } } - if (recId == -1) { - throw new RuntimeException("Could not construct a recoverable key" + - ". This should never happen."); - } - sig.v = (byte) (recId + 27); - return sig; + throw new RuntimeException("Could not construct a recoverable key" + + ". This should never happen."); } @@ -842,10 +848,10 @@ public boolean equals(Object o) { ECKey ecKey = (ECKey) o; - if (privKey != null && !privKey.equals(ecKey.privKey)) { + if (!Objects.equals(privKey, ecKey.privKey)) { return false; } - return pub == null || pub.equals(ecKey.pub); + return Objects.equals(pub, ecKey.pub); } @Override @@ -879,11 +885,6 @@ public ECDSASignature(byte[] r, byte[] s, byte v) { this.v = v; } - /** - * t - * - * @return - - */ private static ECDSASignature fromComponents(byte[] r, byte[] s) { return new ECDSASignature(new BigInteger(1, r), new BigInteger(1, s)); @@ -938,7 +939,7 @@ public ECDSASignature toCanonicalised() { // are valid solutions. // 10 - 8 == 2, giving us always the latter solution, // which is canonical. - return new ECDSASignature(r, CURVE.getN().subtract(s)); + return new ECDSASignature(r, SECP256K1N.subtract(s)); } else { return this; } @@ -953,7 +954,7 @@ public String toBase64() { sigData[0] = v; System.arraycopy(ByteUtil.bigIntegerToBytes(this.r, 32), 0, sigData, 1, 32); System.arraycopy(ByteUtil.bigIntegerToBytes(this.s, 32), 0, sigData, 33, 32); - return new String(Base64.encode(sigData), Charset.forName("UTF-8")); + return new String(Base64.encode(sigData), StandardCharsets.UTF_8); } diff --git a/crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java b/crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java index 63237db9c9e..215b6f06947 100644 --- a/crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java +++ b/crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java @@ -6,14 +6,14 @@ import java.io.Serializable; import java.math.BigInteger; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.security.PrivateKey; import java.security.SecureRandom; import java.security.SignatureException; import java.security.interfaces.ECPrivateKey; -import java.security.interfaces.ECPublicKey; import java.security.spec.InvalidKeySpecException; import java.util.Arrays; +import java.util.Objects; import javax.annotation.Nullable; import lombok.extern.slf4j.Slf4j; import org.bouncycastle.asn1.x9.X9IntegerConverter; @@ -46,23 +46,23 @@ @Slf4j(topic = "crypto") public class SM2 implements Serializable, SignInterface { - private static BigInteger SM2_N = new BigInteger( + private static final BigInteger SM2_N = new BigInteger( "FFFFFFFEFFFFFFFFFFFFFFFFFFFFFFFF7203DF6B21C6052B53BBF40939D54123", 16); - private static BigInteger SM2_P = new BigInteger( + private static final BigInteger SM2_P = new BigInteger( "FFFFFFFEFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00000000FFFFFFFFFFFFFFFF", 16); - private static BigInteger SM2_A = new BigInteger( + private static final BigInteger SM2_A = new BigInteger( "FFFFFFFEFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00000000FFFFFFFFFFFFFFFC", 16); - private static BigInteger SM2_B = new BigInteger( + private static final BigInteger SM2_B = new BigInteger( "28E9FA9E9D9F5E344D5A9E4BCF6509A7F39789F515AB8F92DDBCBD414D940E93", 16); - private static BigInteger SM2_GX = new BigInteger( + private static final BigInteger SM2_GX = new BigInteger( "32C4AE2C1F1981195F9904466A39C9948FE30BBFF2660BE1715A4589334C74C7", 16); - private static BigInteger SM2_GY = new BigInteger( + private static final BigInteger SM2_GY = new BigInteger( "BC3736A2F4F6779C59BDCEE36B692153D0A9877CC62A474002DF32E52139F0A0", 16); - private static ECDomainParameters eccParam; - private static ECParameterSpec eccSpec; - private static ECCurve.Fp curve; - private static ECPoint eccPointG; + private static final ECDomainParameters eccParam; + private static final ECParameterSpec eccSpec; + private static final ECCurve.Fp curve; + private static final ECPoint eccPointG; private static final SecureRandom secureRandom; @@ -80,8 +80,8 @@ public class SM2 implements Serializable, SignInterface { // Transient because it's calculated on demand. - private transient byte[] pubKeyHash; - private transient byte[] nodeId; + private transient volatile byte[] pubKeyHash; + private transient volatile byte[] nodeId; public SM2() { @@ -128,8 +128,12 @@ public SM2(byte[] key, boolean isPrivateKey) { throw new IllegalArgumentException("Empty public key in SM2."); } + ECPoint point = eccParam.getCurve().decodePoint(key); + if (point.isInfinity() || !point.isValid()) { + throw new IllegalArgumentException("Public key is not a valid point on SM2 curve."); + } this.privKey = null; - this.pub = eccParam.getCurve().decodePoint(key); + this.pub = point; } } @@ -140,7 +144,6 @@ public SM2(byte[] key, boolean isPrivateKey) { */ public SM2(@Nullable PrivateKey privKey, ECPoint pub) { - if (privKey == null || isECPrivateKey(privKey)) { this.privKey = privKey; } else { @@ -153,7 +156,7 @@ public SM2(@Nullable PrivateKey privKey, ECPoint pub) { } if (pub == null) { - throw new IllegalArgumentException("Public key may not be null"); + throw new IllegalArgumentException("Public key should not be null"); } else { this.pub = pub; } @@ -164,7 +167,7 @@ public SM2(@Nullable PrivateKey privKey, ECPoint pub) { */ public SM2(@Nullable BigInteger priv, ECPoint pub) { this( - privateKeyFromBigInteger(priv), + priv == null ? null : privateKeyFromBigInteger(priv), pub ); } @@ -198,17 +201,6 @@ private static boolean isECPrivateKey(PrivateKey privKey) { .equals("EC"); } - /* Convert a Java JCE ECPublicKey into a BouncyCastle ECPoint - */ - private static ECPoint extractPublicKey(final ECPublicKey ecPublicKey) { - final java.security.spec.ECPoint publicPointW = ecPublicKey.getW(); - final BigInteger xCoord = publicPointW.getAffineX(); - final BigInteger yCoord = publicPointW.getAffineY(); - - return eccParam.getCurve().createPoint(xCoord, yCoord); - } - - /** * Utility for compressing an elliptic curve point. Returns the same point if it's already * compressed. See the ECKey class docs for a discussion of point compression. @@ -408,7 +400,7 @@ public byte[] getAddress() { if (pubKeyHash == null) { pubKeyHash = computeAddress(this.pub); } - return pubKeyHash; + return Arrays.copyOf(pubKeyHash, pubKeyHash.length); } @@ -482,22 +474,7 @@ public SM2Signature sign(byte[] messageHash) { BigInteger[] componets = signer.generateHashSignature(messageHash); SM2Signature sig = new SM2Signature(componets[0], componets[1]); - // Now we have to work backwards to figure out the recId needed to - // recover the signature. - int recId = -1; - byte[] thisKey = this.pub.getEncoded(/* compressed */ false); - for (int i = 0; i < 4; i++) { - byte[] k = recoverPubBytesFromSignature(i, sig, messageHash); - if (k != null && Arrays.equals(k, thisKey)) { - recId = i; - break; - } - } - if (recId == -1) { - throw new RuntimeException("Could not construct a recoverable key" + - ". This should never happen."); - } - sig.v = (byte) (recId + 27); + sig.v = (byte) (findRecId(sig, messageHash) + 27); return sig; } @@ -528,25 +505,10 @@ public byte[] Base64toBytes(String signature) { */ public SM2Signature signMessage(byte[] message, @Nullable String userID) { SM2Signature sig = signMsg(message, userID); - // Now we have to work backwards to figure out the recId needed to - // recover the signature. - int recId = -1; - byte[] thisKey = this.pub.getEncoded(/* compressed */ false); SM2Signer signer = getSigner(); byte[] messageHash = signer.generateSM3Hash(message); - for (int i = 0; i < 4; i++) { - byte[] k = recoverPubBytesFromSignature(i, sig, messageHash); - if (k != null && Arrays.equals(k, thisKey)) { - recId = i; - break; - } - } - if (recId == -1) { - throw new RuntimeException("Could not construct a recoverable key" + - ". This should never happen."); - } - sig.v = (byte) (recId + 27); + sig.v = (byte) (findRecId(sig, messageHash) + 27); return sig; } @@ -568,6 +530,18 @@ public SM2Signature signMsg(byte[] msg, @Nullable String userID) { return new SM2Signature(componets[0], componets[1]); } + private int findRecId(SM2Signature sig, byte[] messageHash) { + byte[] thisKey = this.pub.getEncoded(/* compressed */ false); + for (int i = 0; i < 4; i++) { + byte[] k = recoverPubBytesFromSignature(i, sig, messageHash); + if (k != null && Arrays.equals(k, thisKey)) { + return i; + } + } + throw new RuntimeException("Could not construct a recoverable key" + + ". This should never happen."); + } + private SM2Signer getSigner() { SM2Signer signer = new SM2Signer(); BigInteger d = getPrivKey(); @@ -623,9 +597,6 @@ public static byte[] recoverPubBytesFromSignature(int recId, // // More concisely, what these points mean is to use X as a compressed // public key. - ECCurve.Fp curve = (ECCurve.Fp) eccParam.getCurve(); - // Bouncy Castle is not consistent - // about the letter it uses for the prime. if (x.compareTo(prime) >= 0) { // Cannot have point co-ordinates larger than this as everything // takes place modulo Q. @@ -789,7 +760,7 @@ public byte[] getNodeId() { if (nodeId == null) { nodeId = pubBytesWithoutFormat(this.pub); } - return nodeId; + return Arrays.copyOf(nodeId, nodeId.length); } @@ -820,9 +791,7 @@ public BigInteger getPrivKey() { } public String toString() { - StringBuilder b = new StringBuilder(); - b.append("pub:").append(Hex.toHexString(pub.getEncoded(false))); - return b.toString(); + return "pub:" + Hex.toHexString(pub.getEncoded(false)); } /** @@ -875,10 +844,10 @@ public boolean equals(Object o) { SM2 ecKey = (SM2) o; - if (privKey != null && !privKey.equals(ecKey.privKey)) { + if (!Objects.equals(privKey, ecKey.privKey)) { return false; } - return pub == null || pub.equals(ecKey.pub); + return Objects.equals(pub, ecKey.pub); } @Override @@ -913,11 +882,6 @@ public SM2Signature(byte[] r, byte[] s, byte v) { this.v = v; } - /** - * t - * - * @return - - */ private static SM2Signature fromComponents(byte[] r, byte[] s) { return new SM2Signature(new BigInteger(1, r), new BigInteger(1, s)); @@ -969,7 +933,7 @@ public String toBase64() { sigData[0] = v; System.arraycopy(bigIntegerToBytes(this.r, 32), 0, sigData, 1, 32); System.arraycopy(bigIntegerToBytes(this.s, 32), 0, sigData, 33, 32); - return new String(Base64.encode(sigData), Charset.forName("UTF-8")); + return new String(Base64.encode(sigData), StandardCharsets.UTF_8); }