diff --git a/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java b/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java index 74d332c5611..005ee839bff 100644 --- a/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java +++ b/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java @@ -941,6 +941,21 @@ public static void validator(DynamicPropertiesStore dynamicPropertiesStore, } break; } + case ALLOW_STRICT_ECDSA_VALIDATION: { + if (!forkController.pass(ForkBlockVersionEnum.VERSION_4_8_3)) { + throw new ContractValidateException( + "Bad chain parameter id [ALLOW_STRICT_ECDSA_VALIDATION]"); + } + if (dynamicPropertiesStore.allowStrictEcdsaValidation()) { + throw new ContractValidateException( + "[ALLOW_STRICT_ECDSA_VALIDATION] has been valid, no need to propose again"); + } + if (value != 1) { + throw new ContractValidateException( + "This value[ALLOW_STRICT_ECDSA_VALIDATION] is only allowed to be 1"); + } + break; + } default: break; } @@ -1029,7 +1044,8 @@ public enum ProposalType { // current value, value range ALLOW_TVM_PRAGUE(95), // 0, 1 ALLOW_TVM_OSAKA(96), // 0, 1 ALLOW_HARDEN_RESOURCE_CALCULATION(97), // 0, 1 - ALLOW_HARDEN_EXCHANGE_CALCULATION(98); // 0, 1 + ALLOW_HARDEN_EXCHANGE_CALCULATION(98), // 0, 1 + ALLOW_STRICT_ECDSA_VALIDATION(99); // 0, 1 private long code; ProposalType(long code) { diff --git a/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java b/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java index 3993e8ed835..833c33eb7cf 100644 --- a/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java +++ b/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java @@ -379,7 +379,8 @@ private static byte[] recoverAddrBySign(byte[] sign, byte[] hash) { CommonParameter.getInstance().isECKeyCryptoEngine()); if (signature.validateComponents()) { out = SignUtils.signatureToAddress(hash, signature, - CommonParameter.getInstance().isECKeyCryptoEngine()); + CommonParameter.getInstance().isECKeyCryptoEngine(), + VMConfig.allowStrictEcdsaValidation()); } } catch (Throwable any) { logger.info("ECRecover error", any.getMessage()); @@ -616,8 +617,9 @@ public Pair execute(byte[] data) { SignatureInterface signature = SignUtils.fromComponents(r, s, v[31] , CommonParameter.getInstance().isECKeyCryptoEngine()); if (validateV(v) && signature.validateComponents()) { - out = new DataWord(SignUtils.signatureToAddress(h, signature - , CommonParameter.getInstance().isECKeyCryptoEngine())); + out = new DataWord(SignUtils.signatureToAddress(h, signature, + CommonParameter.getInstance().isECKeyCryptoEngine(), + VMConfig.allowStrictEcdsaValidation())); } } catch (Throwable any) { } diff --git a/actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java b/actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java index 35480935742..cbf2ec6b6b2 100644 --- a/actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java +++ b/actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java @@ -50,6 +50,7 @@ public static void load(StoreFactory storeFactory, boolean isolate) { snapshot.allowTvmSelfdestructRestriction = ds.getAllowTvmSelfdestructRestriction() == 1; snapshot.allowTvmOsaka = ds.getAllowTvmOsaka() == 1; snapshot.allowHardenResourceCalculation = ds.getAllowHardenResourceCalculation() == 1; + snapshot.allowStrictEcdsaValidation = ds.allowStrictEcdsaValidation(); if (isolate) { VMConfig.setLocalSnapshot(snapshot); } else { diff --git a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java index e6cbd52e595..24ab0a3999d 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java @@ -186,10 +186,17 @@ private Sha256Hash getRawHash() { public boolean validateSignature(DynamicPropertiesStore dynamicPropertiesStore, AccountStore accountStore) throws ValidateSignatureException { try { + ByteString witnessSignature = block.getBlockHeader().getWitnessSignature(); + boolean strictEcdsaValidation = CommonParameter.getInstance().isECKeyCryptoEngine() + && dynamicPropertiesStore.allowStrictEcdsaValidation(); + if (strictEcdsaValidation + && !SignUtils.isValidLength(witnessSignature.size())) { + throw new ValidateSignatureException("Invalid ECDSA signature format"); + } byte[] sigAddress = SignUtils.signatureToAddress(getRawHash().getBytes(), TransactionCapsule.getBase64FromByteString( - block.getBlockHeader().getWitnessSignature()), - CommonParameter.getInstance().isECKeyCryptoEngine()); + witnessSignature), + CommonParameter.getInstance().isECKeyCryptoEngine(), strictEcdsaValidation); byte[] witnessAccountAddress = block.getBlockHeader().getRawData().getWitnessAddress() .toByteArray(); 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 b3f560541cf..1aff9bedb02 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java @@ -100,8 +100,8 @@ public class TransactionCapsule implements ProtoCapsule { private static final long SLOW_SIG_VERIFY_MS = 50; private Transaction transaction; - @Setter - private boolean isVerified = false; + @Getter + private volatile boolean isVerified = false; @Setter @Getter private long blockNum = -1; @@ -233,6 +233,12 @@ public static long getWeight(Permission permission, byte[] address) { public static long checkWeight(Permission permission, List sigs, byte[] hash, List approveList) throws SignatureException, PermissionException, SignatureFormatException { + return checkWeight(permission, sigs, hash, approveList, false); + } + + public static long checkWeight(Permission permission, List sigs, byte[] hash, + List approveList, boolean strictEcdsaValidation) + throws SignatureException, PermissionException, SignatureFormatException { long currentWeight = 0; if (sigs.size() > permission.getKeysCount()) { throw new PermissionException( @@ -241,13 +247,15 @@ public static long checkWeight(Permission permission, List sigs, byt } HashMap addMap = new HashMap(); for (ByteString sig : sigs) { - if (sig.size() < 65) { + if (sig.size() < 65 + || (strictEcdsaValidation && !SignUtils.isValidLength(sig.size()))) { throw new SignatureFormatException( "Signature size is " + sig.size()); } String base64 = TransactionCapsule.getBase64FromByteString(sig); byte[] address = SignUtils - .signatureToAddress(hash, base64, CommonParameter.getInstance().isECKeyCryptoEngine()); + .signatureToAddress(hash, base64, CommonParameter.getInstance().isECKeyCryptoEngine(), + strictEcdsaValidation); long weight = getWeight(permission, address); if (weight == 0) { throw new PermissionException( @@ -488,7 +496,10 @@ public static boolean validateSignature(Transaction transaction, throw new PermissionException("permission isn't exit"); } checkPermission(permissionId, permission, contract); - long weight = checkWeight(permission, transaction.getSignatureList(), hash, null); + boolean strictEcdsaValidation = CommonParameter.getInstance().isECKeyCryptoEngine() + && dynamicPropertiesStore.allowStrictEcdsaValidation(); + long weight = checkWeight(permission, transaction.getSignatureList(), hash, null, + strictEcdsaValidation); if (weight >= permission.getThreshold()) { return true; } @@ -695,7 +706,7 @@ void logSlowSigVerify(long startNs) { /** * validate signature */ - public boolean validateSignature(AccountStore accountStore, + public synchronized boolean validateSignature(AccountStore accountStore, DynamicPropertiesStore dynamicPropertiesStore) throws ValidateSignatureException { if (!isVerified) { //Do not support multi contracts in one transaction @@ -718,6 +729,10 @@ public boolean validateSignature(AccountStore accountStore, return true; } + public synchronized void setVerified(boolean verified) { + isVerified = verified; + } + public Sha256Hash getTransactionId() { if (this.id == null) { this.id = getRawHash(); diff --git a/chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java b/chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java index 0f74f20d379..b0e92f097fc 100644 --- a/chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java +++ b/chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java @@ -255,6 +255,9 @@ public class DynamicPropertiesStore extends TronStoreWithRevoking private static final byte[] ALLOW_HARDEN_EXCHANGE_CALCULATION = "ALLOW_HARDEN_EXCHANGE_CALCULATION".getBytes(); + private static final byte[] ALLOW_STRICT_ECDSA_VALIDATION = + "ALLOW_STRICT_ECDSA_VALIDATION".getBytes(); + private static final byte[] TURKISH_KEY_MIGRATION_DONE = "TURKISH_KEY_MIGRATION_DONE".getBytes(); @@ -3071,6 +3074,21 @@ public boolean allowHardenExchangeCalculation() { return getAllowHardenExchangeCalculation() == 1L; } + public long getAllowStrictEcdsaValidation() { + return Optional.ofNullable(getUnchecked(ALLOW_STRICT_ECDSA_VALIDATION)) + .map(BytesCapsule::getData) + .map(ByteArray::toLong) + .orElse(0L); + } + + public void saveAllowStrictEcdsaValidation(long value) { + this.put(ALLOW_STRICT_ECDSA_VALIDATION, new BytesCapsule(ByteArray.fromLong(value))); + } + + public boolean allowStrictEcdsaValidation() { + return getAllowStrictEcdsaValidation() == 1L; + } + public void saveTurkishKeyMigrationDone(long num) { this.put(TURKISH_KEY_MIGRATION_DONE, new BytesCapsule(ByteArray.fromLong(num))); diff --git a/common/src/main/java/org/tron/core/Constant.java b/common/src/main/java/org/tron/core/Constant.java index 5d3f3099c91..f1b8d8ea33a 100644 --- a/common/src/main/java/org/tron/core/Constant.java +++ b/common/src/main/java/org/tron/core/Constant.java @@ -20,7 +20,6 @@ public class Constant { public static final long TRANSACTION_DEFAULT_EXPIRATION_TIME = 60 * 1_000L; //60 seconds public static final long TRANSACTION_FEE_POOL_PERIOD = 1; //1 blocks public static final int PER_SIGN_LENGTH = 65; - public static final int MAX_PER_SIGN_LENGTH = 68; public static final long MAX_CONTRACT_RESULT_SIZE = 2L; // Smart contract / Energy diff --git a/common/src/main/java/org/tron/core/config/Parameter.java b/common/src/main/java/org/tron/core/config/Parameter.java index 233f1d9ef7a..99919c0ffec 100644 --- a/common/src/main/java/org/tron/core/config/Parameter.java +++ b/common/src/main/java/org/tron/core/config/Parameter.java @@ -30,7 +30,8 @@ public enum ForkBlockVersionEnum { VERSION_4_8_0_1(33, 1596780000000L, 70), VERSION_4_8_1(34, 1596780000000L, 80), VERSION_4_8_1_1(35, 1596780000000L, 70), - VERSION_4_8_2(36, 1596780000000L, 80); + VERSION_4_8_2(36, 1596780000000L, 80), + VERSION_4_8_3(37, 1596780000000L, 80); // if add a version, modify BLOCK_VERSION simultaneously @Getter @@ -79,7 +80,7 @@ public class ChainConstant { public static final int SINGLE_REPEAT = 1; public static final int BLOCK_FILLED_SLOTS_NUMBER = 128; public static final int MAX_FROZEN_NUMBER = 1; - public static final int BLOCK_VERSION = 36; + public static final int BLOCK_VERSION = 37; public static final long FROZEN_PERIOD = 86_400_000L; public static final long DELEGATE_PERIOD = 3 * 86_400_000L; public static final long TRX_PRECISION = 1000_000L; diff --git a/common/src/main/java/org/tron/core/vm/config/VMConfig.java b/common/src/main/java/org/tron/core/vm/config/VMConfig.java index 304ced33698..e179415cd76 100644 --- a/common/src/main/java/org/tron/core/vm/config/VMConfig.java +++ b/common/src/main/java/org/tron/core/vm/config/VMConfig.java @@ -46,6 +46,7 @@ public static class Snapshot { public boolean allowTvmSelfdestructRestriction; public boolean allowTvmOsaka; public boolean allowHardenResourceCalculation; + public boolean allowStrictEcdsaValidation; } // HEAD / block-processing config, written by the consensus path; read by everyone with no @@ -204,6 +205,10 @@ public static void initAllowHardenResourceCalculation(long allow) { globalSnapshot.allowHardenResourceCalculation = allow == 1; } + public static void initAllowStrictEcdsaValidation(long allow) { + globalSnapshot.allowStrictEcdsaValidation = allow == 1; + } + public static boolean getEnergyLimitHardFork() { return CommonParameter.ENERGY_LIMIT_HARD_FORK; } @@ -311,4 +316,8 @@ public static boolean allowTvmOsaka() { public static boolean allowHardenResourceCalculation() { return current().allowHardenResourceCalculation; } + + public static boolean allowStrictEcdsaValidation() { + return current().allowStrictEcdsaValidation; + } } 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..4ae429f20ad 100644 --- a/crypto/src/main/java/org/tron/common/crypto/ECKey.java +++ b/crypto/src/main/java/org/tron/common/crypto/ECKey.java @@ -49,6 +49,7 @@ import org.bouncycastle.math.ec.ECAlgorithms; import org.bouncycastle.math.ec.ECCurve; import org.bouncycastle.math.ec.ECPoint; +import org.bouncycastle.util.BigIntegers; import org.bouncycastle.util.encoders.Base64; import org.bouncycastle.util.encoders.Hex; import org.tron.common.crypto.jce.ECKeyFactory; @@ -386,6 +387,11 @@ public static ECKey fromNodeId(byte[] nodeId) { public static byte[] signatureToKeyBytes(byte[] messageHash, String signatureBase64) throws SignatureException { + return signatureToKeyBytes(messageHash, signatureBase64, false); + } + + public static byte[] signatureToKeyBytes(byte[] messageHash, String + signatureBase64, boolean strictValidation) throws SignatureException { byte[] signatureEncoded; try { signatureEncoded = Base64.decode(signatureBase64); @@ -395,9 +401,10 @@ public static byte[] signatureToKeyBytes(byte[] messageHash, String throw new SignatureException("Could not decode base64", e); } // Parse the signature bytes into r/s and the selector value. - if (signatureEncoded.length < 65) { - throw new SignatureException("Signature truncated, expected 65 " + - "bytes and got " + signatureEncoded.length); + if (signatureEncoded.length < 65 + || (strictValidation && signatureEncoded.length != 65)) { + throw new SignatureException("Signature has invalid length " + signatureEncoded.length + + ", expected " + (strictValidation ? "exactly " : "at least ") + "65 bytes"); } return signatureToKeyBytes( @@ -405,11 +412,17 @@ public static byte[] signatureToKeyBytes(byte[] messageHash, String ECDSASignature.fromComponents( Arrays.copyOfRange(signatureEncoded, 1, 33), Arrays.copyOfRange(signatureEncoded, 33, 65), - (byte) (signatureEncoded[0] & 0xFF))); + (byte) (signatureEncoded[0] & 0xFF)), + strictValidation); } public static byte[] signatureToKeyBytes(byte[] messageHash, ECDSASignature sig) throws SignatureException { + return signatureToKeyBytes(messageHash, sig, false); + } + + public static byte[] signatureToKeyBytes(byte[] messageHash, + ECDSASignature sig, boolean strictValidation) throws SignatureException { check(messageHash.length == 32, "messageHash argument has length " + messageHash.length); int header = sig.v; @@ -425,7 +438,7 @@ public static byte[] signatureToKeyBytes(byte[] messageHash, } int recId = header - 27; byte[] key = ECKey.recoverPubBytesFromSignature(recId, sig, - messageHash); + messageHash, strictValidation); if (key == null) { throw new SignatureException("Could not recover public key from " + "signature"); @@ -442,8 +455,13 @@ public static byte[] signatureToKeyBytes(byte[] messageHash, */ public static byte[] signatureToAddress(byte[] messageHash, String signatureBase64) throws SignatureException { + return signatureToAddress(messageHash, signatureBase64, false); + } + + public static byte[] signatureToAddress(byte[] messageHash, String + signatureBase64, boolean strictValidation) throws SignatureException { return Hash.computeAddress(signatureToKeyBytes(messageHash, - signatureBase64)); + signatureBase64, strictValidation)); } /** @@ -456,7 +474,12 @@ public static byte[] signatureToAddress(byte[] messageHash, String public static byte[] signatureToAddress(byte[] messageHash, ECDSASignature sig) throws SignatureException { - return Hash.computeAddress(signatureToKeyBytes(messageHash, sig)); + return signatureToAddress(messageHash, sig, false); + } + + public static byte[] signatureToAddress(byte[] messageHash, + ECDSASignature sig, boolean strictValidation) throws SignatureException { + return Hash.computeAddress(signatureToKeyBytes(messageHash, sig, strictValidation)); } /** @@ -517,6 +540,14 @@ public static boolean isPubKeyCanonical(byte[] pubkey) { @Nullable public static byte[] recoverPubBytesFromSignature(int recId, ECDSASignature sig, byte[] messageHash) { + return recoverPubBytesFromSignature(recId, sig, messageHash, false); + } + + @Nullable + public static byte[] recoverPubBytesFromSignature(int recId, + ECDSASignature sig, byte[] messageHash, boolean strictValidation) { + check(sig != null && sig.r != null && sig.s != null, + "signature and its components must not be null"); 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"); @@ -525,6 +556,14 @@ public static byte[] recoverPubBytesFromSignature(int recId, // this function) // 1.1 Let x = r + jn BigInteger n = CURVE.getN(); // Curve order. + if (strictValidation) { + check(recId <= 3, "recId must be in the range [0, 3]"); + check(sig.r.signum() > 0 && sig.r.compareTo(n) < 0, + "r must be in the range [1, n)"); + check(sig.s.signum() > 0 && sig.s.compareTo(n) < 0, + "s must be in the range [1, n)"); + check(messageHash.length == 32, "messageHash must be 32 bytes"); + } BigInteger i = BigInteger.valueOf((long) recId / 2); BigInteger x = sig.r.add(i.multiply(n)); // 1.2. Convert the integer x to an octet string X of length mlen @@ -577,11 +616,16 @@ public static byte[] recoverPubBytesFromSignature(int recId, // 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 rInv = strictValidation + ? BigIntegers.modOddInverse(n, sig.r) + : sig.r.modInverse(n); BigInteger srInv = rInv.multiply(sig.s).mod(n); BigInteger eInvrInv = rInv.multiply(eInv).mod(n); ECPoint.Fp q = (ECPoint.Fp) ECAlgorithms.sumOfTwoMultiplies(CURVE .getG(), eInvrInv, R, srInv); + if (strictValidation && q.isInfinity()) { + return null; + } return q.getEncoded(/* compressed */ false); } @@ -940,7 +984,6 @@ public static boolean validateComponents(BigInteger r, BigInteger s, return BIUtil.isLessThan(s, SECP256K1N); } - public boolean validateComponents() { return validateComponents(r, s, v); } diff --git a/crypto/src/main/java/org/tron/common/crypto/SignUtils.java b/crypto/src/main/java/org/tron/common/crypto/SignUtils.java index e0e20fb2677..cd5e26a923e 100644 --- a/crypto/src/main/java/org/tron/common/crypto/SignUtils.java +++ b/crypto/src/main/java/org/tron/common/crypto/SignUtils.java @@ -1,6 +1,5 @@ package org.tron.common.crypto; -import static org.tron.core.Constant.MAX_PER_SIGN_LENGTH; import static org.tron.core.Constant.PER_SIGN_LENGTH; import java.security.SecureRandom; @@ -13,17 +12,15 @@ public class SignUtils { /** * Strict signature-length check for admission entry-points (RPC broadcast, - * P2P transaction ingress, peer hello handshake). Accepts only sizes in - * [{@link org.tron.core.Constant#PER_SIGN_LENGTH PER_SIGN_LENGTH}, - * {@link org.tron.core.Constant#MAX_PER_SIGN_LENGTH MAX_PER_SIGN_LENGTH}]. + * P2P transaction ingress, peer hello handshake). Accepts exactly + * {@link org.tron.core.Constant#PER_SIGN_LENGTH PER_SIGN_LENGTH} bytes. * - *

Consensus paths (e.g. {@code TransactionCapsule.checkWeight}) intentionally - * keep the looser {@code size < 65} check to remain compatible with historical - * on-chain signatures that carry trailing padding bytes; do not call this - * helper from those paths. + *

Consensus paths use the governance-controlled strict-validation flag. Their + * legacy behavior remains compatible with historical on-chain signatures that + * carry trailing padding bytes. */ public static boolean isValidLength(int size) { - return size >= PER_SIGN_LENGTH && size <= MAX_PER_SIGN_LENGTH; + return size == PER_SIGN_LENGTH; } public static SignInterface getGeneratedRandomSign( @@ -44,9 +41,15 @@ public static SignInterface fromPrivate(byte[] privKeyBytes, boolean isECKeyCryp public static byte[] signatureToAddress( byte[] messageHash, String signatureBase64, boolean isECKeyCryptoEngine) throws SignatureException { + return signatureToAddress(messageHash, signatureBase64, isECKeyCryptoEngine, false); + } + + public static byte[] signatureToAddress( + byte[] messageHash, String signatureBase64, boolean isECKeyCryptoEngine, + boolean strictEcdsaValidation) throws SignatureException { try { if (isECKeyCryptoEngine) { - return ECKey.signatureToAddress(messageHash, signatureBase64); + return ECKey.signatureToAddress(messageHash, signatureBase64, strictEcdsaValidation); } return SM2.signatureToAddress(messageHash, signatureBase64); } catch (Exception e) { @@ -65,8 +68,15 @@ public static SignatureInterface fromComponents( public static byte[] signatureToAddress( byte[] messageHash, SignatureInterface signatureInterface, boolean isECKeyCryptoEngine) throws SignatureException { + return signatureToAddress(messageHash, signatureInterface, isECKeyCryptoEngine, false); + } + + public static byte[] signatureToAddress( + byte[] messageHash, SignatureInterface signatureInterface, boolean isECKeyCryptoEngine, + boolean strictEcdsaValidation) throws SignatureException { if (isECKeyCryptoEngine) { - return ECKey.signatureToAddress(messageHash, (ECDSASignature) signatureInterface); + return ECKey.signatureToAddress(messageHash, (ECDSASignature) signatureInterface, + strictEcdsaValidation); } return SM2.signatureToAddress(messageHash, (SM2Signature) signatureInterface); } diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index ac54cb2b7ff..d13f1cf1a1a 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -1524,6 +1524,11 @@ public Protocol.ChainParameters getChainParameters() { .setValue(dbManager.getDynamicPropertiesStore().getAllowHardenExchangeCalculation()) .build()); + builder.addChainParameter(Protocol.ChainParameters.ChainParameter.newBuilder() + .setKey("getAllowStrictEcdsaValidation") + .setValue(dbManager.getDynamicPropertiesStore().getAllowStrictEcdsaValidation()) + .build()); + return builder.build(); } diff --git a/framework/src/main/java/org/tron/core/consensus/ProposalService.java b/framework/src/main/java/org/tron/core/consensus/ProposalService.java index 543deab2fc6..04b8793a2f3 100644 --- a/framework/src/main/java/org/tron/core/consensus/ProposalService.java +++ b/framework/src/main/java/org/tron/core/consensus/ProposalService.java @@ -412,6 +412,11 @@ public static boolean process(Manager manager, ProposalCapsule proposalCapsule) .saveAllowHardenExchangeCalculation(entry.getValue()); break; } + case ALLOW_STRICT_ECDSA_VALIDATION: { + manager.getDynamicPropertiesStore() + .saveAllowStrictEcdsaValidation(entry.getValue()); + break; + } default: find = false; break; diff --git a/framework/src/main/java/org/tron/core/db/Manager.java b/framework/src/main/java/org/tron/core/db/Manager.java index 9d7a7c979b9..03195cbbf1d 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -1260,7 +1260,9 @@ public List getVerifyTxs(BlockCapsule block) { block.getTransactions().forEach(capsule -> { String address = Hex.toHexString(capsule.getOwnerAddress()); String txId = Hex.toHexString(capsule.getTransactionId().getBytes()); - if (multiAddresses.contains(address) || !isSameSig(capsule, txMap.get(txId))) { + TransactionCapsule pendingTx = txMap.get(txId); + if (multiAddresses.contains(address) || pendingTx == null || !pendingTx.isVerified() + || !isSameSig(capsule, pendingTx)) { txs.add(capsule); } else { capsule.setVerified(true); @@ -1919,7 +1921,14 @@ private void processBlock(BlockCapsule block, List txs) boolean flag = chainBaseManager.getDynamicPropertiesStore().getNextMaintenanceTime() <= block.getTimeStamp(); if (flag) { + boolean strictEcdsaValidation = getDynamicPropertiesStore() + .allowStrictEcdsaValidation(); proposalController.processProposals(); + if (!strictEcdsaValidation && getDynamicPropertiesStore() + .allowStrictEcdsaValidation()) { + // Legacy verification results must not survive the consensus-rule activation boundary. + invalidateTransactionVerificationCache(); + } } if (!consensus.applyBlock(block)) { @@ -1943,6 +1952,13 @@ private void processBlock(BlockCapsule block, List txs) block.setBloom(blockBloom); } + private void invalidateTransactionVerificationCache() { + pendingTransactions.forEach(tx -> tx.setVerified(false)); + rePushTransactions.forEach(tx -> tx.setVerified(false)); + poppedTransactions.forEach(tx -> tx.setVerified(false)); + pushTransactionQueue.forEach(tx -> tx.setVerified(false)); + } + private void payReward(BlockCapsule block) { WitnessCapsule witnessCapsule = chainBaseManager.getWitnessStore().getUnchecked(block.getInstance().getBlockHeader() diff --git a/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java b/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java index d4e010ff21d..0c55fd38f26 100644 --- a/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java +++ b/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java @@ -163,7 +163,7 @@ public boolean checkHelloMessage(HelloMessage message, Channel channel) { String sig = TransactionCapsule.getBase64FromByteString(msg.getSignature()); byte[] sigAddress = SignUtils.signatureToAddress(hash.getBytes(), sig, - Args.getInstance().isECKeyCryptoEngine()); + Args.getInstance().isECKeyCryptoEngine(), true); if (manager.getDynamicPropertiesStore().getAllowMultiSign() != 1) { flag = Arrays.equals(sigAddress, msg.getAddress().toByteArray()); } else { 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..153441df4fc 100644 --- a/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java +++ b/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java @@ -6,16 +6,20 @@ 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.nio.charset.StandardCharsets; import java.security.KeyPairGenerator; import java.security.Security; import java.security.SignatureException; import java.util.Arrays; import lombok.extern.slf4j.Slf4j; +import org.bouncycastle.util.BigIntegers; +import org.bouncycastle.util.encoders.Base64; import org.bouncycastle.util.encoders.Hex; import org.junit.Test; import org.tron.common.crypto.ECKey.ECDSASignature; @@ -119,6 +123,110 @@ public void testInvalidSignatureLength() throws SignatureException { fail("Expecting a SignatureException for invalid signature length"); } + @Test + public void testStrictSignatureLength() throws SignatureException { + byte[] messageHash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.ONE); + byte[] signature = Base64.decode(key.signHash(messageHash)); + byte[] padded = Arrays.copyOf(signature, 66); + String paddedBase64 = new String(Base64.encode(padded), StandardCharsets.UTF_8); + + assertArrayEquals(key.getPubKey(), ECKey.signatureToKeyBytes(messageHash, paddedBase64)); + assertThrows(SignatureException.class, + () -> ECKey.signatureToKeyBytes(messageHash, paddedBase64, true)); + } + + @Test + public void testStrictRecoveryBoundsAndHighS() throws SignatureException { + byte[] messageHash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + ECDSASignature lowS = key.sign(messageHash); + int recId = lowS.v - 27; + BigInteger curveOrder = ECKey.CURVE.getN(); + + assertArrayEquals(key.getPubKey(), + ECKey.recoverPubBytesFromSignature(recId, lowS, messageHash, false)); + assertArrayEquals(key.getPubKey(), + ECKey.recoverPubBytesFromSignature(recId, lowS, messageHash, true)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(-1, lowS, messageHash, true)); + assertNull(ECKey.recoverPubBytesFromSignature(4, lowS, messageHash, false)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(4, lowS, messageHash, true)); + + for (BigInteger invalidScalar : Arrays.asList( + BigInteger.ZERO, curveOrder, curveOrder.add(BigInteger.ONE))) { + ECDSASignature invalidR = new ECDSASignature(invalidScalar, lowS.s); + invalidR.v = lowS.v; + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(recId, invalidR, messageHash, true)); + + ECDSASignature invalidS = new ECDSASignature(lowS.r, invalidScalar); + invalidS.v = lowS.v; + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(recId, invalidS, messageHash, true)); + } + + for (byte invalidHeader : new byte[]{26, 35}) { + ECDSASignature invalidSignature = new ECDSASignature(lowS.r, lowS.s); + invalidSignature.v = invalidHeader; + assertThrows(SignatureException.class, + () -> ECKey.signatureToKeyBytes(messageHash, invalidSignature, true)); + } + + ECDSASignature highS = new ECDSASignature( + lowS.r, curveOrder.subtract(lowS.s)); + highS.v = (byte) (27 + (recId ^ 1)); + assertArrayEquals(key.getPubKey(), ECKey.signatureToKeyBytes(messageHash, highS, true)); + } + + @Test + public void testRecoveryRejectsNullSignatureComponents() { + byte[] messageHash = new byte[32]; + + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(0, null, messageHash, false)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(0, + new ECDSASignature(null, BigInteger.ONE), messageHash, false)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(0, + new ECDSASignature(BigInteger.ONE, null), messageHash, false)); + } + + @Test + public void testStrictRecoveryRejectsPointAtInfinity() { + byte[] messageHash = new byte[32]; + messageHash[messageHash.length - 1] = 1; + ECDSASignature signature = new ECDSASignature( + ECKey.CURVE.getG().getAffineXCoord().toBigInteger(), BigInteger.ONE); + signature.v = 27; + + assertArrayEquals(new byte[]{0}, + ECKey.recoverPubBytesFromSignature(0, signature, messageHash, false)); + assertNull(ECKey.recoverPubBytesFromSignature(0, signature, messageHash, true)); + assertThrows(SignatureException.class, + () -> ECKey.signatureToKeyBytes(messageHash, signature, true)); + } + + @Test + public void testModOddInverseConformance() { + BigInteger n = ECKey.CURVE.getN(); + BigInteger[] values = { + BigInteger.ONE, + BigInteger.valueOf(2), + BigInteger.TEN, + new BigInteger("123456789abcdef", 16), + n.subtract(BigInteger.ONE) + }; + + for (BigInteger value : values) { + BigInteger inverse = BigIntegers.modOddInverse(n, value); + assertEquals(value.modInverse(n), inverse); + assertEquals(BigInteger.ONE, value.multiply(inverse).mod(n)); + } + } + @Test public void testPublicKeyFromPrivate() { byte[] pubFromPriv = ECKey.publicKeyFromPrivate(privateKey, false); diff --git a/framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsTest.java b/framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsTest.java index d5a50ea4f9d..5cde2cf97f7 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsTest.java @@ -11,12 +11,14 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.tuple.Pair; import org.bouncycastle.util.Arrays; +import org.bouncycastle.util.BigIntegers; import org.bouncycastle.util.encoders.Hex; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.tron.common.BaseTest; import org.tron.common.TestConstants; +import org.tron.common.crypto.ECKey; import org.tron.common.runtime.ProgramResult; import org.tron.common.utils.ByteArray; import org.tron.common.utils.ByteUtil; @@ -167,6 +169,29 @@ private PrecompiledContract createPrecompiledContract(DataWord addr, String owne return contract; } + @Test + public void testECRecoverPointAtInfinityStrictValidation() { + byte[] input = new byte[128]; + input[31] = 1; + input[63] = 27; + byte[] r = BigIntegers.asUnsignedByteArray( + 32, ECKey.CURVE.getG().getAffineXCoord().toBigInteger()); + System.arraycopy(r, 0, input, 64, r.length); + input[127] = 1; + PrecompiledContract contract = new PrecompiledContracts.ECRecover(); + boolean previousStrictValidation = VMConfig.allowStrictEcdsaValidation(); + + try { + VMConfig.initAllowStrictEcdsaValidation(0); + Assert.assertEquals(32, contract.execute(input).getRight().length); + + VMConfig.initAllowStrictEcdsaValidation(1); + Assert.assertArrayEquals(new byte[0], contract.execute(input).getRight()); + } finally { + VMConfig.initAllowStrictEcdsaValidation(previousStrictValidation ? 1 : 0); + } + } + //@Test public void voteWitnessNativeTest() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, diff --git a/framework/src/test/java/org/tron/common/runtime/vm/VMConfigIsolationTest.java b/framework/src/test/java/org/tron/common/runtime/vm/VMConfigIsolationTest.java index 845db6dd6af..264d65cd02f 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/VMConfigIsolationTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/VMConfigIsolationTest.java @@ -73,6 +73,17 @@ public void testSetGlobalConfigDropsLocalView() { assertFalse("setGlobalSnapshot must drop the thread-local view", VMConfig.allowTvmOsaka()); } + @Test + public void testSnapshotGlobalPreservesStrictEcdsaValidation() { + VMConfig.initAllowStrictEcdsaValidation(1); + VMConfig.Snapshot snapshot = snapshotGlobal(); + + VMConfig.initAllowStrictEcdsaValidation(0); + VMConfig.setGlobalSnapshot(snapshot); + + assertTrue(VMConfig.allowStrictEcdsaValidation()); + } + // Deep-copy the current global config through the public getters (no thread-local set here, so // the getters read the global) so @After can restore the exact prior state. private static VMConfig.Snapshot snapshotGlobal() { @@ -103,6 +114,7 @@ private static VMConfig.Snapshot snapshotGlobal() { snapshot.allowTvmSelfdestructRestriction = VMConfig.allowTvmSelfdestructRestriction(); snapshot.allowTvmOsaka = VMConfig.allowTvmOsaka(); snapshot.allowHardenResourceCalculation = VMConfig.allowHardenResourceCalculation(); + snapshot.allowStrictEcdsaValidation = VMConfig.allowStrictEcdsaValidation(); return snapshot; } } diff --git a/framework/src/test/java/org/tron/core/WalletMockTest.java b/framework/src/test/java/org/tron/core/WalletMockTest.java index 2f4c08d8f9f..5d5444c352f 100644 --- a/framework/src/test/java/org/tron/core/WalletMockTest.java +++ b/framework/src/test/java/org/tron/core/WalletMockTest.java @@ -184,7 +184,7 @@ public void testBroadcastTxInvalidSigLength() throws Exception { GrpcAPI.Return ret = wallet.broadcastTransaction(shortSig); assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode()); - // signature longer than 68 bytes → SIGERROR + // signature longer than 65 bytes → SIGERROR Protocol.Transaction longSig = Protocol.Transaction.newBuilder() .addSignature(ByteString.copyFrom(new byte[69])) .build(); @@ -209,12 +209,12 @@ public void testBroadcastTxInvalidSigLength() throws Exception { ret = wallet.broadcastTransaction(validSig); assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode()); - // 68-byte signature (upper bound) also passes the length check + // padded signatures are rejected even when the first 65 bytes are present Protocol.Transaction paddedSig = Protocol.Transaction.newBuilder() .addSignature(ByteString.copyFrom(new byte[68])) .build(); ret = wallet.broadcastTransaction(paddedSig); - assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode()); + assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode()); } @Test diff --git a/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java b/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java index 16a3cb3a5bb..da3d99e42d5 100644 --- a/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java +++ b/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java @@ -349,6 +349,8 @@ public void validateCheck() { testAllowHardenExchangeCalculationProposal(); + testAllowStrictEcdsaValidationProposal(); + forkUtils.getManager().getDynamicPropertiesStore() .statsByVersion(ForkBlockVersionEnum.ENERGY_LIMIT.getValue(), stats); forkUtils.reset(); @@ -744,6 +746,36 @@ private void testAllowHardenExchangeCalculationProposal() { } } + private void testAllowStrictEcdsaValidationProposal() { + long code = ProposalType.ALLOW_STRICT_ECDSA_VALIDATION.getCode(); + ThrowingRunnable proposeZero = () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, + code, 0); + ThrowingRunnable proposeOne = () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, + code, 1); + + ContractValidateException thrown = assertThrows(ContractValidateException.class, proposeOne); + assertEquals("Bad chain parameter id [ALLOW_STRICT_ECDSA_VALIDATION]", + thrown.getMessage()); + + activateFork(ForkBlockVersionEnum.VERSION_4_8_3); + + thrown = assertThrows(ContractValidateException.class, proposeZero); + assertEquals("This value[ALLOW_STRICT_ECDSA_VALIDATION] is only allowed to be 1", + thrown.getMessage()); + + try { + proposeOne.run(); + } catch (Throwable e) { + Assert.fail("Should allow one-way activation: " + e.getMessage()); + } + + dynamicPropertiesStore.saveAllowStrictEcdsaValidation(1); + thrown = assertThrows(ContractValidateException.class, proposeOne); + assertEquals( + "[ALLOW_STRICT_ECDSA_VALIDATION] has been valid, no need to propose again", + thrown.getMessage()); + } + private void testAllowMarketTransaction() { ThrowingRunnable off = () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, ProposalType.ALLOW_MARKET_TRANSACTION.getCode(), 0); diff --git a/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java b/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java index b258fbf99a1..f81a294e4a6 100644 --- a/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java +++ b/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java @@ -239,6 +239,37 @@ public void testValidateSignatureReturnsTrueWhenSignerMatches() throws Exception Assert.assertTrue(block.validateSignature(dps, accountStore)); } + @Test + public void shouldGateStrictWitnessSignatureLength() throws Exception { + String key = PublicMethod.getRandomPrivateKey(); + byte[] witnessAddress = PublicMethod.getAddressByteByPrivateKey(key); + BlockCapsule block = new BlockCapsule(4, + Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString( + "9938a342238077182498b464ac0292229938a342238077182498b464ac029222"))), + 6789, + ByteString.copyFrom(witnessAddress)); + block.sign(ByteArray.fromHexString(key)); + + ByteString signature = block.getInstance().getBlockHeader().getWitnessSignature(); + BlockHeader paddedHeader = block.getInstance().getBlockHeader().toBuilder() + .setWitnessSignature(signature.concat(ByteString.copyFrom(new byte[3]))) + .build(); + BlockCapsule paddedBlock = new BlockCapsule(block.getInstance().toBuilder() + .setBlockHeader(paddedHeader) + .build()); + + DynamicPropertiesStore dps = mock(DynamicPropertiesStore.class); + when(dps.getAllowMultiSign()).thenReturn(0L); + AccountStore accountStore = mock(AccountStore.class); + + Assert.assertTrue(paddedBlock.validateSignature(dps, accountStore)); + + when(dps.allowStrictEcdsaValidation()).thenReturn(true); + Assert.assertTrue(block.validateSignature(dps, accountStore)); + Assert.assertThrows(ValidateSignatureException.class, + () -> paddedBlock.validateSignature(dps, accountStore)); + } + /** * The other failure mode switchFork must handle: signature bytes are * malformed (cannot recover a public key). validateSignature wraps the diff --git a/framework/src/test/java/org/tron/core/capsule/TransactionCapsuleTest.java b/framework/src/test/java/org/tron/core/capsule/TransactionCapsuleTest.java index 9c2e004931e..3f1ca2878fe 100644 --- a/framework/src/test/java/org/tron/core/capsule/TransactionCapsuleTest.java +++ b/framework/src/test/java/org/tron/core/capsule/TransactionCapsuleTest.java @@ -9,8 +9,13 @@ import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.read.ListAppender; import com.google.protobuf.ByteString; +import java.math.BigInteger; +import java.security.SignatureException; +import java.util.Arrays; import java.util.List; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; import org.junit.Assert; @@ -20,10 +25,18 @@ import org.slf4j.LoggerFactory; import org.tron.common.BaseTest; import org.tron.common.TestConstants; +import org.tron.common.crypto.ECKey; +import org.tron.common.utils.ByteUtil; import org.tron.common.utils.StringUtil; import org.tron.core.Wallet; import org.tron.core.config.args.Args; +import org.tron.core.exception.SignatureFormatException; +import org.tron.core.exception.ValidateSignatureException; +import org.tron.core.store.AccountStore; +import org.tron.core.store.DynamicPropertiesStore; import org.tron.protos.Protocol.AccountType; +import org.tron.protos.Protocol.Key; +import org.tron.protos.Protocol.Permission; import org.tron.protos.Protocol.Transaction; import org.tron.protos.Protocol.Transaction.Contract.ContractType; import org.tron.protos.Protocol.Transaction.Result; @@ -64,6 +77,68 @@ public void trxCapsuleClearTest() { .getRet(0).getContractRet(), Result.contractResult.OUT_OF_TIME); } + @Test + public void shouldGateStrictSignatureLength() throws Exception { + byte[] hash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + Permission permission = permissionFor(key); + ByteString signature = ByteString.copyFrom(key.Base64toBytes(key.signHash(hash))); + ByteString padded = signature.concat(ByteString.copyFrom(new byte[3])); + + Assert.assertEquals(1L, TransactionCapsule.checkWeight( + permission, Arrays.asList(signature), hash, null, true)); + Assert.assertEquals(1L, TransactionCapsule.checkWeight( + permission, Arrays.asList(padded), hash, null, false)); + Assert.assertThrows(SignatureFormatException.class, + () -> TransactionCapsule.checkWeight( + permission, Arrays.asList(padded), hash, null, true)); + } + + @Test + public void shouldRejectInvalidComponentsInStrictMode() { + byte[] hash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + Permission permission = permissionFor(key); + byte[] signature = key.Base64toBytes(key.signHash(hash)); + BigInteger curveOrder = ECKey.CURVE.getN(); + + for (BigInteger invalidScalar : Arrays.asList( + BigInteger.ZERO, curveOrder, curveOrder.add(BigInteger.ONE))) { + assertStrictComponentRejected(permission, hash, + replaceScalar(signature, 0, invalidScalar)); + assertStrictComponentRejected(permission, hash, + replaceScalar(signature, 32, invalidScalar)); + } + + for (byte invalidV : new byte[]{8, 26, 35}) { + byte[] invalidSignature = Arrays.copyOf(signature, signature.length); + invalidSignature[64] = invalidV; + assertStrictComponentRejected(permission, hash, invalidSignature); + } + } + + private byte[] replaceScalar(byte[] signature, int offset, BigInteger scalar) { + byte[] result = Arrays.copyOf(signature, signature.length); + System.arraycopy(ByteUtil.bigIntegerToBytes(scalar, 32), 0, result, offset, 32); + return result; + } + + private void assertStrictComponentRejected(Permission permission, byte[] hash, + byte[] signature) { + Assert.assertThrows(SignatureException.class, + () -> TransactionCapsule.checkWeight(permission, + Arrays.asList(ByteString.copyFrom(signature)), hash, null, true)); + } + + private Permission permissionFor(ECKey key) { + return Permission.newBuilder() + .setThreshold(1) + .addKeys(Key.newBuilder() + .setAddress(ByteString.copyFrom(key.getAddress())) + .setWeight(1)) + .build(); + } + @Test public void testRemoveRedundantRet() { Transaction.Builder transaction = Transaction.newBuilder().setRawData(raw.newBuilder() @@ -134,4 +209,65 @@ public void fastVerify() { capsuleLogger.setLevel(originalLevel); } } + + @Test + public void shouldInvalidateVerificationCacheAfterInFlightValidation() throws Exception { + CountDownLatch validationStarted = new CountDownLatch(1); + CountDownLatch continueValidation = new CountDownLatch(1); + CountDownLatch invalidationCompleted = new CountDownLatch(1); + AtomicReference failure = new AtomicReference<>(); + Transaction transaction = Transaction.newBuilder() + .setRawData(raw.newBuilder() + .addContract(Transaction.Contract.newBuilder() + .setType(ContractType.TransferContract))) + .build(); + TransactionCapsule capsule = new TransactionCapsule(transaction) { + @Override + public boolean validatePubSignature(AccountStore accountStore, + DynamicPropertiesStore dynamicPropertiesStore) throws ValidateSignatureException { + validationStarted.countDown(); + try { + if (!continueValidation.await(5, TimeUnit.SECONDS)) { + throw new ValidateSignatureException("timed out waiting to continue validation"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new ValidateSignatureException("validation interrupted"); + } + return true; + } + }; + + Thread validationThread = new Thread(() -> { + try { + capsule.validateSignature(null, null); + } catch (Throwable t) { + failure.set(t); + } + }); + Thread invalidationThread = new Thread(() -> { + capsule.setVerified(false); + invalidationCompleted.countDown(); + }); + + try { + validationThread.start(); + Assert.assertTrue(validationStarted.await(5, TimeUnit.SECONDS)); + invalidationThread.start(); + Assert.assertFalse(invalidationCompleted.await(100, TimeUnit.MILLISECONDS)); + + continueValidation.countDown(); + validationThread.join(TimeUnit.SECONDS.toMillis(5)); + invalidationThread.join(TimeUnit.SECONDS.toMillis(5)); + + Assert.assertFalse(validationThread.isAlive()); + Assert.assertFalse(invalidationThread.isAlive()); + Assert.assertNull(failure.get()); + Assert.assertFalse(capsule.isVerified()); + } finally { + continueValidation.countDown(); + validationThread.interrupt(); + invalidationThread.interrupt(); + } + } } diff --git a/framework/src/test/java/org/tron/core/db/ManagerMockTest.java b/framework/src/test/java/org/tron/core/db/ManagerMockTest.java index 946bef022d2..4640cc3954a 100644 --- a/framework/src/test/java/org/tron/core/db/ManagerMockTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerMockTest.java @@ -26,6 +26,8 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; @@ -702,6 +704,37 @@ public void testSwitchForkPassesValidSignatureBlockToApply() { verify(goodBlock, atLeastOnce()).setSwitch(true); } + @Test + public void shouldInvalidateTransactionVerificationCacheAcrossAllQueues() + throws Exception { + Manager manager = new Manager(); + TransactionCapsule pendingTx = mock(TransactionCapsule.class); + TransactionCapsule rePushTx = mock(TransactionCapsule.class); + TransactionCapsule poppedTx = mock(TransactionCapsule.class); + TransactionCapsule pushingTx = mock(TransactionCapsule.class); + BlockingQueue pending = new LinkedBlockingQueue<>(); + BlockingQueue rePush = new LinkedBlockingQueue<>(); + List popped = new ArrayList<>(); + BlockingQueue pushing = new LinkedBlockingQueue<>(); + pending.add(pendingTx); + rePush.add(rePushTx); + popped.add(poppedTx); + pushing.add(pushingTx); + setField(manager, "pendingTransactions", pending); + setField(manager, "rePushTransactions", rePush); + setField(manager, "poppedTransactions", popped); + setField(manager, "pushTransactionQueue", pushing); + + Method method = Manager.class.getDeclaredMethod("invalidateTransactionVerificationCache"); + method.setAccessible(true); + method.invoke(manager); + + verify(pendingTx).setVerified(false); + verify(rePushTx).setVerified(false); + verify(poppedTx).setVerified(false); + verify(pushingTx).setVerified(false); + } + private static void setField(Object target, String name, Object value) throws Exception { Field f = target.getClass().getSuperclass() != null ? findField(target.getClass(), name) @@ -722,4 +755,4 @@ private static Field findField(Class cls, String name) throws NoSuchFieldExce throw new NoSuchFieldException(name); } -} \ No newline at end of file +} diff --git a/framework/src/test/java/org/tron/core/db/ManagerTest.java b/framework/src/test/java/org/tron/core/db/ManagerTest.java index 958a132fbbf..25dea4e88bd 100755 --- a/framework/src/test/java/org/tron/core/db/ManagerTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerTest.java @@ -844,10 +844,16 @@ public void getVerifyTxsTest() { List txs = dbManager.getVerifyTxs(capsule); Assert.assertEquals(txs.size(), 1); + t1.setVerified(true); dbManager.getPendingTransactions().add(t1); txs = dbManager.getVerifyTxs(capsule); Assert.assertEquals(txs.size(), 0); + t1.setVerified(false); + txs = dbManager.getVerifyTxs(capsule); + Assert.assertEquals(txs.size(), 1); + t1.setVerified(true); + list.add(t2.getInstance()); capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); txs = dbManager.getVerifyTxs(capsule); @@ -859,6 +865,7 @@ public void getVerifyTxsTest() { dbManager.getPendingTransactions().clear(); capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); + t2.setVerified(true); dbManager.getPendingTransactions().add(t1); dbManager.getPendingTransactions().add(t2); txs = dbManager.getVerifyTxs(capsule); diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java index ed2121d360f..b93cc1e2890 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java @@ -368,7 +368,7 @@ public void testInvalidSigLength() throws Exception { () -> handler.processMessage(peer, new TransactionsMessage(shortList))); Assert.assertEquals(TypeEnum.BAD_TRX, shortEx.getType()); - // signature longer than 68 bytes → BAD_TRX + // signature longer than 65 bytes → BAD_TRX Protocol.Transaction longSigTrx = Protocol.Transaction.newBuilder() .setRawData(Protocol.Transaction.raw.newBuilder() .setRefBlockNum(1) @@ -402,7 +402,7 @@ public void testInvalidSigLength() throws Exception { stubAdvInvRequest(peer, new TransactionsMessage(validList)); handler.processMessage(peer, new TransactionsMessage(validList)); - // 68 bytes (upper bound) also passes the length check + // padded signatures are rejected Protocol.Transaction paddedSigTrx = Protocol.Transaction.newBuilder() .setRawData(Protocol.Transaction.raw.newBuilder() .setRefBlockNum(3) @@ -416,7 +416,9 @@ public void testInvalidSigLength() throws Exception { List paddedList = new ArrayList<>(); paddedList.add(paddedSigTrx); stubAdvInvRequest(peer, new TransactionsMessage(paddedList)); - handler.processMessage(peer, new TransactionsMessage(paddedList)); + P2pException paddedEx = Assert.assertThrows(P2pException.class, + () -> handler.processMessage(peer, new TransactionsMessage(paddedList))); + Assert.assertEquals(TypeEnum.BAD_TRX, paddedEx.getType()); } finally { handler.close(); } diff --git a/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java b/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java index 5732e6f1cde..ba2f6b2c763 100644 --- a/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java +++ b/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java @@ -134,6 +134,19 @@ public void testUpdateConsensusLogicOptimization() { Assert.assertTrue(dbManager.getDynamicPropertiesStore().disableJavaLangMath()); } + @Test + public void shouldActivateStrictEcdsaValidation() { + dbManager.getDynamicPropertiesStore().saveAllowStrictEcdsaValidation(0); + Proposal proposal = Proposal.newBuilder() + .putParameters(ProposalType.ALLOW_STRICT_ECDSA_VALIDATION.getCode(), 1) + .build(); + + Assert.assertTrue(ProposalService.process(dbManager, new ProposalCapsule(proposal))); + Assert.assertEquals(1L, + dbManager.getDynamicPropertiesStore().getAllowStrictEcdsaValidation()); + Assert.assertTrue(dbManager.getDynamicPropertiesStore().allowStrictEcdsaValidation()); + } + @Test public void testProposalExpireTime() { long defaultWindow = dbManager.getDynamicPropertiesStore().getProposalExpireTime(); @@ -151,4 +164,4 @@ public void testProposalExpireTime() { Assert.assertEquals(MAX_PROPOSAL_EXPIRE_TIME - 3000, window); } -} \ No newline at end of file +}