From 2caafc96820506b4eb10b9873286135eeacf105a Mon Sep 17 00:00:00 2001 From: federico Date: Tue, 4 Aug 2026 15:00:51 +0800 Subject: [PATCH 1/2] fix(crypto): harden ECKey validation --- .../java/org/tron/common/crypto/ECKey.java | 265 ++++++++---------- .../main/java/org/tron/keystore/Wallet.java | 37 ++- .../org/tron/common/crypto/ECKeyTest.java | 120 +++++++- .../args/WitnessInitializerKeystoreTest.java | 22 ++ .../keystore/WalletAddressValidationTest.java | 61 ++++ 5 files changed, 332 insertions(+), 173 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 d0a6048aca1..ddb25f6afa1 100644 --- a/crypto/src/main/java/org/tron/common/crypto/ECKey.java +++ b/crypto/src/main/java/org/tron/common/crypto/ECKey.java @@ -23,15 +23,11 @@ import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.PrivateKey; -import java.security.Provider; import java.security.PublicKey; 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.sec.SECNamedCurves; @@ -58,6 +54,14 @@ import org.tron.common.utils.ByteArray; import org.tron.common.utils.ByteUtil; +/** + * A secp256k1 key pair and ECDSA signing utility. + * + *

ECDSA signatures are malleable: for a valid {@code (r, s)} pair, {@code (r, n - s)} is + * valid as well. New signatures are canonicalized to low-S form, but verification accepts both + * forms because transaction identifiers do not include signatures. See BIP-62 for background on + * low-S signatures. + */ @Slf4j(topic = "crypto") public class ECKey implements Serializable, SignInterface { @@ -77,6 +81,9 @@ public class ECKey implements Serializable, SignInterface { */ public static final BigInteger HALF_CURVE_ORDER; + private static final int MAX_PRIVATE_KEY_LENGTH = 32; + private static final int COMPRESSED_PUBLIC_KEY_LENGTH = 33; + private static final int UNCOMPRESSED_PUBLIC_KEY_LENGTH = 65; private static final BigInteger SECP256K1N = new BigInteger("fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", 16); private static final SecureRandom secureRandom; @@ -100,16 +107,10 @@ public class ECKey implements Serializable, SignInterface { // TODO: Redesign this class to use consistent internals and more // efficient serialization. private final PrivateKey privKey; - // the Java Cryptographic Architecture provider to use for Signature - // this is set along with the PrivateKey privKey and must be compatible - // this provider will be used when selecting a Signature instance - // https://docs.oracle.com/javase/8/docs/technotes/guides/security - // /SunProviders.html - 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. @@ -121,78 +122,58 @@ public ECKey() { } /** - * Generate a new keypair using the given Java Security Provider. + * Generates an entirely new keypair with the given {@link SecureRandom} object.

BouncyCastle + * will be used as the Java Security Provider * - *

All private key operations will use the provider. + * @param secureRandom - */ - public ECKey(Provider provider, SecureRandom secureRandom) { - this.provider = provider; - - final KeyPairGenerator keyPairGen = ECKeyPairGenerator.getInstance(provider, secureRandom); + public ECKey(SecureRandom secureRandom) { + final KeyPairGenerator keyPairGen = ECKeyPairGenerator.getInstance( + TronCastleProvider.getInstance(), secureRandom); final KeyPair keyPair = keyPairGen.generateKeyPair(); this.privKey = keyPair.getPrivate(); final PublicKey pubKey = keyPair.getPublic(); - if (pubKey instanceof BCECPublicKey) { - pub = ((BCECPublicKey) pubKey).getQ(); - } else if (pubKey instanceof ECPublicKey) { - pub = extractPublicKey((ECPublicKey) pubKey); - } else { + if (!(pubKey instanceof BCECPublicKey)) { throw new AssertionError( - "Expected Provider " + provider.getName() - + " to produce a subtype of ECPublicKey, found " - + pubKey.getClass()); + "Expected Bouncy Castle EC public key, found " + pubKey.getClass()); } + this.pub = ((BCECPublicKey) pubKey).getQ(); } - /** - * Generates an entirely new keypair with the given {@link SecureRandom} object.

BouncyCastle - * will be used as the Java Security Provider - * - * @param secureRandom - - */ - public ECKey(SecureRandom secureRandom) { - this(TronCastleProvider.getInstance(), secureRandom); - } - - /** - * Pair a private key with a public EC point. - * - *

All private key operations will use the provider. - */ - public ECKey(byte[] key, boolean isPrivateKey) { if (isPrivateKey) { + check(isValidPrivateKey(key), "Invalid private key"); BigInteger pk = new BigInteger(1, key); this.privKey = privateKeyFromBigInteger(pk); this.pub = CURVE.getG().multiply(pk); } else { this.privKey = null; - this.pub = CURVE.getCurve().decodePoint(key); + this.pub = decodePublicKey(key); + check(isValidPublicPoint(this.pub), "Invalid public key"); } - this.provider = TronCastleProvider.getInstance(); } - public ECKey(Provider provider, @Nullable PrivateKey privKey, ECPoint pub) { - this.provider = provider; + private ECKey(BigInteger privateKey) { + this.privKey = privateKeyFromBigInteger(privateKey); + this.pub = CURVE.getG().multiply(privateKey); + } - if (privKey == null || isECPrivateKey(privKey)) { + private ECKey(@Nullable PrivateKey privKey, ECPoint pub) { + if (privKey == null || privKey instanceof BCECPrivateKey) { this.privKey = privKey; } else { throw new IllegalArgumentException( - "Expected EC private key, given a private key object with" + - " class " + "Expected Bouncy Castle EC private key, given a private key object with class " + privKey.getClass().toString() + " and algorithm " + privKey.getAlgorithm()); } - if (pub == null) { - throw new IllegalArgumentException("Public key may not be null"); - } else { - this.pub = pub; - } + check(isValidPublicPoint(pub), "Invalid public key"); + checkPrivateKeyMatchesPublic(privKey, pub); + this.pub = pub; } /** @@ -200,32 +181,17 @@ public ECKey(Provider provider, @Nullable PrivateKey privKey, ECPoint pub) { * Security Provider */ public ECKey(@Nullable BigInteger priv, ECPoint pub) { - this( - TronCastleProvider.getInstance(), - privateKeyFromBigInteger(priv), - pub - ); + this(privateKeyFromBigInteger(priv), pub); } - /* 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 CURVE.getCurve().createPoint(xCoord, yCoord); - } - - /* Test if a generic private key is an EC private key - * - * it is not sufficient to check that privKey is a subtype of ECPrivateKey - * as the SunPKCS11 Provider will return a generic PrivateKey instance - * a fallback that covers this case is to check the key algorithm - */ - private static boolean isECPrivateKey(PrivateKey privKey) { - return privKey instanceof ECPrivateKey || privKey.getAlgorithm() - .equals("EC"); + private static void checkPrivateKeyMatchesPublic( + @Nullable PrivateKey privateKey, ECPoint publicPoint) { + if (privateKey instanceof BCECPrivateKey) { + BigInteger privateScalar = ((BCECPrivateKey) privateKey).getD(); + check(isValidPrivateKey(privateScalar), "Invalid private key"); + check(CURVE.getG().multiply(privateScalar).equals(publicPoint), + "Private key does not match public key"); + } } /* Convert a BigInteger into a PrivateKey object @@ -234,6 +200,7 @@ private static PrivateKey privateKeyFromBigInteger(BigInteger priv) { if (priv == null) { return null; } else { + check(isValidPrivateKey(priv), "Invalid private key"); try { return ECKeyFactory .getInstance(TronCastleProvider.getInstance()) @@ -245,6 +212,57 @@ private static PrivateKey privateKeyFromBigInteger(BigInteger priv) { } } + /** + * Returns whether the supplied key bytes represent a valid secp256k1 private scalar. + * Accepts unsigned encodings up to 32 bytes and Java {@link BigInteger} encodings with one + * leading zero sign byte. + */ + public static boolean isValidPrivateKey(byte[] privateKey) { + // BigInteger.toByteArray() prepends a zero sign byte only when the magnitude's high bit is set. + return !ByteArray.isEmpty(privateKey) + && (privateKey.length <= MAX_PRIVATE_KEY_LENGTH + || (privateKey.length == MAX_PRIVATE_KEY_LENGTH + 1 + && privateKey[0] == 0 + && (privateKey[1] & 0x80) != 0)) + && isValidPrivateKey(new BigInteger(1, privateKey)); + } + + /** + * Returns whether the supplied value is in the secp256k1 private-key range {@code [1, n)}. + */ + public static boolean isValidPrivateKey(BigInteger privateKey) { + return privateKey != null + && privateKey.signum() > 0 + && privateKey.compareTo(SECP256K1N) < 0; + } + + /** + * Returns whether the encoded key is a non-infinity point on its elliptic curve. + */ + public static boolean isValidPublicKey(byte[] publicKey) { + try { + return isValidPublicPoint(decodePublicKey(publicKey)); + } catch (RuntimeException e) { + return false; + } + } + + private static boolean isValidPublicPoint(ECPoint publicPoint) { + return publicPoint != null + && CURVE.getCurve().equals(publicPoint.getCurve()) + && !publicPoint.isInfinity() + && publicPoint.isValid(); + } + + private static ECPoint decodePublicKey(byte[] publicKey) { + check(isPubKeyCanonical(publicKey), "Invalid public key"); + try { + return CURVE.getCurve().decodePoint(publicKey); + } catch (RuntimeException e) { + throw new IllegalArgumentException("Invalid public key", e); + } + } + /** * 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,7 +294,8 @@ public static ECPoint decompressPoint(ECPoint compressed) { * @return - */ public static ECKey fromPrivate(BigInteger privKey) { - return new ECKey(privKey, CURVE.getG().multiply(privKey)); + check(isValidPrivateKey(privKey), "Invalid private key"); + return new ECKey(privKey); } /** @@ -286,41 +305,8 @@ public static ECKey fromPrivate(BigInteger privKey) { * @return - */ public static ECKey fromPrivate(byte[] privKeyBytes) { - if (ByteArray.isEmpty(privKeyBytes)) { - return null; - } - 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)); + check(isValidPrivateKey(privKeyBytes), "Invalid private key"); + return new ECKey(new BigInteger(1, privKeyBytes)); } /** @@ -331,7 +317,7 @@ public static ECKey fromPrivateAndPrecalculatedPublic(byte[] priv, byte[] * @return - */ public static ECKey fromPublicOnly(ECPoint pub) { - return new ECKey(null, pub); + return new ECKey((PrivateKey) null, pub); } /** @@ -342,7 +328,7 @@ public static ECKey fromPublicOnly(ECPoint pub) { * @return - */ public static ECKey fromPublicOnly(byte[] pub) { - return new ECKey(null, CURVE.getCurve().decodePoint(pub)); + return new ECKey((PrivateKey) null, decodePublicKey(pub)); } /** @@ -355,6 +341,7 @@ public static ECKey fromPublicOnly(byte[] pub) { */ public static byte[] publicKeyFromPrivate(BigInteger privKey, boolean compressed) { + check(isValidPrivateKey(privKey), "Invalid private key"); ECPoint point = CURVE.getG().multiply(privKey); return point.getEncoded(compressed); } @@ -481,12 +468,15 @@ public static ECKey signatureToKey(byte[] messageHash, String * @return - */ public static boolean isPubKeyCanonical(byte[] pubkey) { + if (ByteArray.isEmpty(pubkey)) { + return false; + } if (pubkey[0] == 0x04) { // Uncompressed pubkey - return pubkey.length == 65; + return pubkey.length == UNCOMPRESSED_PUBLIC_KEY_LENGTH; } else if (pubkey[0] == 0x02 || pubkey[0] == 0x03) { // Compressed pubkey - return pubkey.length == 33; + return pubkey.length == COMPRESSED_PUBLIC_KEY_LENGTH; } else { return false; } @@ -669,10 +659,12 @@ public boolean hasPrivKey() { * @return 21-byte address */ public byte[] getAddress() { - if (pubKeyHash == null) { - pubKeyHash = Hash.computeAddress(this.pub); + byte[] address = pubKeyHash; + if (address == null) { + address = Hash.computeAddress(this.pub); + pubKeyHash = address; } - return pubKeyHash; + return Arrays.copyOf(address, address.length); } @Override @@ -691,10 +683,12 @@ public byte[] Base64toBytes(String signature) { * Generates the NodeID based on this key, that is the public key without first format byte */ public byte[] getNodeId() { - if (nodeId == null) { - nodeId = pubBytesWithoutFormat(this.pub); + byte[] id = nodeId; + if (id == null) { + id = pubBytesWithoutFormat(this.pub); + nodeId = id; } - return nodeId; + return Arrays.copyOf(id, id.length); } @@ -744,22 +738,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 @@ -841,8 +819,6 @@ public byte[] getPrivKeyBytes() { return null; } else if (privKey instanceof BCECPrivateKey) { return ByteUtil.bigIntegerToBytes(((BCECPrivateKey) privKey).getD(), 32); - } else if (privKey instanceof ECPrivateKey) { - return ByteUtil.bigIntegerToBytes(((ECPrivateKey) privKey).getS(), 32); } else { return null; } @@ -860,10 +836,7 @@ public boolean equals(Object o) { ECKey ecKey = (ECKey) o; - if (privKey != null && !privKey.equals(ecKey.privKey)) { - return false; - } - return pub == null || pub.equals(ecKey.pub); + return pub.equals(ecKey.pub); } @Override diff --git a/crypto/src/main/java/org/tron/keystore/Wallet.java b/crypto/src/main/java/org/tron/keystore/Wallet.java index d63525b1e4d..127d724a567 100644 --- a/crypto/src/main/java/org/tron/keystore/Wallet.java +++ b/crypto/src/main/java/org/tron/keystore/Wallet.java @@ -216,22 +216,31 @@ public static SignInterface decrypt(String password, WalletFile walletFile, byte[] encryptKey = Arrays.copyOfRange(derivedKey, 0, 16); byte[] privateKey = performCipherOperation(Cipher.DECRYPT_MODE, iv, encryptKey, cipherText); - SignInterface keyPair = SignUtils.fromPrivate(privateKey, ecKey); - - // Enforce address consistency: if the keystore declares an address, it MUST match - // the address derived from the decrypted private key. Prevents address spoofing - // where a crafted keystore displays one address but encrypts a different key. - String declared = walletFile.getAddress(); - if (declared != null && !declared.isEmpty()) { - String derived = StringUtil.encode58Check(keyPair.getAddress()); - if (!declared.equals(derived)) { - throw new CipherException( - "Keystore address mismatch: file declares " + declared - + " but private key derives " + derived); + try { + SignInterface keyPair; + try { + keyPair = SignUtils.fromPrivate(privateKey, ecKey); + } catch (IllegalArgumentException e) { + throw new CipherException("Invalid private key in keystore", e); } - } - return keyPair; + // Enforce address consistency: if the keystore declares an address, it MUST match + // the address derived from the decrypted private key. Prevents address spoofing + // where a crafted keystore displays one address but encrypts a different key. + String declared = walletFile.getAddress(); + if (declared != null && !declared.isEmpty()) { + String derived = StringUtil.encode58Check(keyPair.getAddress()); + if (!declared.equals(derived)) { + throw new CipherException( + "Keystore address mismatch: file declares " + declared + + " but private key derives " + derived); + } + } + + return keyPair; + } finally { + Arrays.fill(privateKey, (byte) 0); + } } /** 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..6f983f9e66d 100644 --- a/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java +++ b/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java @@ -5,17 +5,16 @@ 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; import java.math.BigInteger; -import java.security.KeyPairGenerator; -import java.security.Security; import java.security.SignatureException; import java.util.Arrays; import lombok.extern.slf4j.Slf4j; +import org.bouncycastle.asn1.sec.SECNamedCurves; import org.bouncycastle.util.encoders.Hex; import org.junit.Test; import org.tron.common.crypto.ECKey.ECDSASignature; @@ -69,10 +68,76 @@ 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 + public void shouldValidatePrivateKeyRange() { + assertTrue(ECKey.isValidPrivateKey(privateKey)); + assertTrue(ECKey.isValidPrivateKey(Hex.decode(privString))); + assertFalse(ECKey.isValidPrivateKey((BigInteger) null)); + assertFalse(ECKey.isValidPrivateKey((byte[]) null)); + assertFalse(ECKey.isValidPrivateKey(new byte[0])); + assertFalse(ECKey.isValidPrivateKey(new byte[33])); + assertFalse(ECKey.isValidPrivateKey(BigInteger.ZERO)); + assertFalse(ECKey.isValidPrivateKey(ECKey.CURVE.getN())); + + BigInteger highBitPrivateKey = ECKey.CURVE.getN().subtract(BigInteger.ONE); + byte[] signPaddedPrivateKey = highBitPrivateKey.toByteArray(); + assertEquals(33, signPaddedPrivateKey.length); + assertEquals(0, signPaddedPrivateKey[0]); + assertTrue(ECKey.isValidPrivateKey(signPaddedPrivateKey)); + assertEquals(highBitPrivateKey, ECKey.fromPrivate(signPaddedPrivateKey).getPrivKey()); + + byte[] redundantSignPaddedPrivateKey = new byte[33]; + redundantSignPaddedPrivateKey[32] = 1; + assertFalse(ECKey.isValidPrivateKey(redundantSignPaddedPrivateKey)); + + byte[] nonZeroLeadingPrivateKey = Arrays.copyOf(signPaddedPrivateKey, + signPaddedPrivateKey.length); + nonZeroLeadingPrivateKey[0] = 1; + assertFalse(ECKey.isValidPrivateKey(nonZeroLeadingPrivateKey)); + + byte[] doubleSignPaddedPrivateKey = new byte[34]; + System.arraycopy(signPaddedPrivateKey, 0, doubleSignPaddedPrivateKey, 1, + signPaddedPrivateKey.length); + assertFalse(ECKey.isValidPrivateKey(doubleSignPaddedPrivateKey)); + + assertThrows(IllegalArgumentException.class, + () -> ECKey.fromPrivate(BigInteger.ZERO)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.fromPrivate(ECKey.CURVE.getN())); + assertThrows(IllegalArgumentException.class, + () -> ECKey.fromPrivate(new byte[1024 * 1024])); + assertThrows(IllegalArgumentException.class, + () -> ECKey.publicKeyFromPrivate(null, false)); + } + + @Test + public void shouldRejectInvalidPublicKeys() { + assertTrue(ECKey.isValidPublicKey(pubKey)); + assertTrue(ECKey.isValidPublicKey(compressedPubKey)); + assertFalse(ECKey.isValidPublicKey(null)); + assertFalse(ECKey.isValidPublicKey(new byte[0])); + assertFalse(ECKey.isValidPublicKey(new byte[]{0})); + assertFalse(ECKey.isValidPublicKey(new byte[66])); + + byte[] oversizedPublicKey = new byte[1024 * 1024]; + oversizedPublicKey[0] = 0x04; + assertFalse(ECKey.isValidPublicKey(oversizedPublicKey)); + + assertThrows(IllegalArgumentException.class, + () -> ECKey.fromPublicOnly((byte[]) null)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.fromPublicOnly(ECKey.CURVE.getCurve().getInfinity())); + assertThrows(IllegalArgumentException.class, + () -> ECKey.fromPublicOnly( + ECKey.CURVE.getCurve().createPoint(BigInteger.ZERO, BigInteger.ZERO))); + assertThrows(IllegalArgumentException.class, + () -> ECKey.fromPublicOnly(SECNamedCurves.getByName("secp256r1").getG())); } @Test(expected = IllegalArgumentException.class) @@ -81,12 +146,21 @@ public void testPrivatePublicKeyBytesNoArg() { fail("Expecting an IllegalArgumentException for using only null-parameters"); } - @Test(expected = IllegalArgumentException.class) - public void testInvalidPrivateKey() throws Exception { - new ECKey(Security.getProvider("SunEC"), - KeyPairGenerator.getInstance("RSA").generateKeyPair().getPrivate(), - ECKey.fromPublicOnly(pubKey).getPubKeyPoint()); - fail("Expecting an IllegalArgumentException for using an non EC private key"); + @Test + public void shouldRejectMismatchedPrivateAndPublicKeys() { + BigInteger otherPrivateKey = privateKey.add(BigInteger.ONE); + ECKey otherKey = ECKey.fromPrivate(otherPrivateKey); + + assertThrows(IllegalArgumentException.class, + () -> new ECKey(privateKey, otherKey.getPubKeyPoint())); + } + + @Test + public void shouldAcceptMatchingPrivateAndPublicKeys() { + ECKey key = new ECKey(privateKey, ECKey.CURVE.getG().multiply(privateKey)); + + assertArrayEquals(pubKey, key.getPubKey()); + assertTrue(key.hasPrivKey()); } @Test @@ -201,6 +275,8 @@ public void testIsPubKeyCanonicalWrongPrefix() { // Test wrong prefix 3, right length 33 byte[] nonCanonicalPubkey6 = new byte[33]; assertFalse(ECKey.isPubKeyCanonical(nonCanonicalPubkey6)); + assertFalse(ECKey.isPubKeyCanonical(null)); + assertFalse(ECKey.isPubKeyCanonical(new byte[0])); } @Test @@ -215,10 +291,28 @@ public void testEqualsObject() { ECKey key0 = new ECKey(); ECKey key1 = ECKey.fromPrivate(privateKey); ECKey key2 = ECKey.fromPrivate(privateKey); + ECKey publicOnlyKey = ECKey.fromPublicOnly(key1.getPubKey()); assertFalse(key0.equals(key1)); assertTrue(key1.equals(key1)); assertTrue(key1.equals(key2)); + assertTrue(key1.equals(publicOnlyKey)); + assertTrue(publicOnlyKey.equals(key1)); + assertEquals(key1.hashCode(), publicOnlyKey.hashCode()); + } + + @Test + public void shouldReturnDefensiveCopiesOfCachedValues() { + ECKey key = ECKey.fromPrivate(privateKey); + byte[] expectedAddress = Arrays.copyOf(key.getAddress(), key.getAddress().length); + byte[] returnedAddress = key.getAddress(); + returnedAddress[0] ^= 1; + assertArrayEquals(expectedAddress, key.getAddress()); + + byte[] expectedNodeId = Arrays.copyOf(key.getNodeId(), key.getNodeId().length); + byte[] returnedNodeId = key.getNodeId(); + returnedNodeId[0] ^= 1; + assertArrayEquals(expectedNodeId, key.getNodeId()); } diff --git a/framework/src/test/java/org/tron/core/config/args/WitnessInitializerKeystoreTest.java b/framework/src/test/java/org/tron/core/config/args/WitnessInitializerKeystoreTest.java index 80d8287682b..386ac7c90a9 100644 --- a/framework/src/test/java/org/tron/core/config/args/WitnessInitializerKeystoreTest.java +++ b/framework/src/test/java/org/tron/core/config/args/WitnessInitializerKeystoreTest.java @@ -5,6 +5,9 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mockStatic; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.spi.ILoggingEvent; @@ -18,11 +21,13 @@ import org.junit.ClassRule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.mockito.MockedStatic; import org.slf4j.LoggerFactory; import org.tron.common.crypto.SignInterface; import org.tron.common.crypto.SignUtils; import org.tron.common.utils.ByteArray; import org.tron.common.utils.LocalWitnesses; +import org.tron.core.exception.CipherException; import org.tron.core.exception.TronError; import org.tron.keystore.Credentials; import org.tron.keystore.WalletFile; @@ -183,6 +188,23 @@ public void testTamperedKeystoreRejectedAtSrLoading() throws Exception { } } + @Test + public void testInvalidPrivateKeyLoadError() { + java.util.List keystores = + java.util.Collections.singletonList(keystoreFileName); + + try (MockedStatic mockedSignUtils = mockStatic(SignUtils.class)) { + mockedSignUtils.when(() -> SignUtils.fromPrivate(any(byte[].class), eq(true))) + .thenThrow(new IllegalArgumentException("Invalid private key")); + + TronError err = assertThrows(TronError.class, + () -> WitnessInitializer.initFromKeystore(keystores, PASSWORD, null)); + assertEquals(TronError.ErrCode.WITNESS_KEYSTORE_LOAD, err.getErrCode()); + assertTrue(err.getCause() instanceof CipherException); + assertTrue(err.getCause().getCause() instanceof IllegalArgumentException); + } + } + private static ListAppender attachAppender() { ListAppender appender = new ListAppender<>(); appender.start(); diff --git a/framework/src/test/java/org/tron/keystore/WalletAddressValidationTest.java b/framework/src/test/java/org/tron/keystore/WalletAddressValidationTest.java index 82008988b6e..75cce11328e 100644 --- a/framework/src/test/java/org/tron/keystore/WalletAddressValidationTest.java +++ b/framework/src/test/java/org/tron/keystore/WalletAddressValidationTest.java @@ -1,11 +1,19 @@ package org.tron.keystore; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mockStatic; +import java.util.concurrent.atomic.AtomicReference; import org.junit.Test; +import org.mockito.MockedStatic; import org.tron.common.crypto.SignInterface; import org.tron.common.crypto.SignUtils; import org.tron.common.utils.Utils; @@ -90,4 +98,57 @@ public void testDecryptRejectsSpoofedAddressSm2() throws Exception { assertTrue(e.getMessage().contains("address mismatch")); } } + + @Test + public void testDecryptClearsPrivateKey() throws Exception { + String password = "test123456"; + SignInterface keyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true); + WalletFile walletFile = Wallet.createLight(password, keyPair); + byte[] expectedPrivateKey = keyPair.getPrivateKey(); + AtomicReference decryptedPrivateKey = new AtomicReference<>(); + + try (MockedStatic mockedSignUtils = mockStatic(SignUtils.class)) { + mockedSignUtils.when(() -> SignUtils.fromPrivate(any(byte[].class), eq(true))) + .thenAnswer(invocation -> { + byte[] privateKey = invocation.getArgument(0); + assertArrayEquals(expectedPrivateKey, privateKey); + decryptedPrivateKey.set(privateKey); + return keyPair; + }); + + assertSame(keyPair, Wallet.decrypt(password, walletFile, true)); + } + + byte[] clearedPrivateKey = decryptedPrivateKey.get(); + assertNotNull(clearedPrivateKey); + assertArrayEquals(new byte[clearedPrivateKey.length], clearedPrivateKey); + } + + @Test + public void testDecryptWrapsInvalidPrivateKey() throws Exception { + String password = "test123456"; + SignInterface keyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true); + WalletFile walletFile = Wallet.createLight(password, keyPair); + byte[] expectedPrivateKey = keyPair.getPrivateKey(); + AtomicReference decryptedPrivateKey = new AtomicReference<>(); + + try (MockedStatic mockedSignUtils = mockStatic(SignUtils.class)) { + mockedSignUtils.when(() -> SignUtils.fromPrivate(any(byte[].class), eq(true))) + .thenAnswer(invocation -> { + byte[] privateKey = invocation.getArgument(0); + assertArrayEquals(expectedPrivateKey, privateKey); + decryptedPrivateKey.set(privateKey); + throw new IllegalArgumentException("Invalid private key"); + }); + + CipherException exception = assertThrows(CipherException.class, + () -> Wallet.decrypt(password, walletFile, true)); + assertTrue(exception.getMessage().contains("Invalid private key")); + assertTrue(exception.getCause() instanceof IllegalArgumentException); + } + + byte[] clearedPrivateKey = decryptedPrivateKey.get(); + assertNotNull(clearedPrivateKey); + assertArrayEquals(new byte[clearedPrivateKey.length], clearedPrivateKey); + } } From 28d5aafae344dbe893b36f10043125eb42bbcea9 Mon Sep 17 00:00:00 2001 From: federico Date: Thu, 13 Aug 2026 17:40:35 +0800 Subject: [PATCH 2/2] test(crypto): simplify ECKey test names --- .../test/java/org/tron/common/crypto/ECKeyTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 6f983f9e66d..f847ef30bd8 100644 --- a/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java +++ b/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java @@ -75,7 +75,7 @@ public void testFromPrivateKey() { } @Test - public void shouldValidatePrivateKeyRange() { + public void testValidatePrivateKey() { assertTrue(ECKey.isValidPrivateKey(privateKey)); assertTrue(ECKey.isValidPrivateKey(Hex.decode(privString))); assertFalse(ECKey.isValidPrivateKey((BigInteger) null)); @@ -117,7 +117,7 @@ public void shouldValidatePrivateKeyRange() { } @Test - public void shouldRejectInvalidPublicKeys() { + public void testRejectInvalidPublicKey() { assertTrue(ECKey.isValidPublicKey(pubKey)); assertTrue(ECKey.isValidPublicKey(compressedPubKey)); assertFalse(ECKey.isValidPublicKey(null)); @@ -147,7 +147,7 @@ public void testPrivatePublicKeyBytesNoArg() { } @Test - public void shouldRejectMismatchedPrivateAndPublicKeys() { + public void testRejectMismatchedKeyPair() { BigInteger otherPrivateKey = privateKey.add(BigInteger.ONE); ECKey otherKey = ECKey.fromPrivate(otherPrivateKey); @@ -156,7 +156,7 @@ public void shouldRejectMismatchedPrivateAndPublicKeys() { } @Test - public void shouldAcceptMatchingPrivateAndPublicKeys() { + public void testAcceptMatchingKeyPair() { ECKey key = new ECKey(privateKey, ECKey.CURVE.getG().multiply(privateKey)); assertArrayEquals(pubKey, key.getPubKey()); @@ -302,7 +302,7 @@ public void testEqualsObject() { } @Test - public void shouldReturnDefensiveCopiesOfCachedValues() { + public void testDefensiveCopy() { ECKey key = ECKey.fromPrivate(privateKey); byte[] expectedAddress = Arrays.copyOf(key.getAddress(), key.getAddress().length); byte[] returnedAddress = key.getAddress();